floccusaddon / floccusaddon/floccus

XBEL items with no @id attribute get NaN ids, causing duplicated folders and false-positive deletion failsafe

Open
#2,322 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
8.5k
Forks
327
Avg merge
3d 14h
Merged PRs (30d)
2

Description

### Version of floccus
5.10.2

### How many bookmarks do you have (approximately)?
Reproduced with as few as 4 (isolated test), but the same mechanism was observed in production with several hundred, where it tripped the failsafe at 100%.

### Do you use any other means of syncing your bookmarks momentarily?
No

### How do you sync your bookmarks?
WebDAV

### Browser and version
Firefox (current stable), Floccus WebDAV account

### Which WebDAV server do you use?
Reproduced with `marlluslustosa/floccuswebdav` (nginx WsgiDAV) test container, and independently observed against a self-hosted lo-floccus/WebDAV setup in production. Not specific to one WebDAV server — the bug is in Floccus's own XBEL parsing, not the transport.

### Describe the Bug
`XbelSerializer._parseFolder` (`src/lib/serializers/Xbel.ts`) parses an item's id with plain `parseInt(node[':@']['@_id'])`. If the ``/`` element has no `@id` attribute at all (or a non-numeric one), `parseInt(undefined)` returns `NaN`, and that `NaN` is later serialized back to the file as the literal string `id="NaN"` (`String(NaN)`).

Because `NaN !== NaN` in JavaScript, any subsequent sync that tries to match this item against the cache/local tree by id can never recognize it as "the same item" — every comparison involving that id fails. In our reproduction this caused Floccus to treat the same folder as brand-new on each sync and duplicate it (we saw one folder triplicated across 3 sync cycles). Once the local/cache/server trees disagree about that much of the tree, the diff looks like a mass deletion, and the "would delete N% of your local links" failsafe (E050) fires — we reproduced it at 40% in the small isolated case; in the wild, on a bigger tree with more affected folders, it can reach much higher percentages (we saw 100% in production).

Folders/bookmarks end up with no `@id` in the first place whenever something writes the XBEL file without one (e.g. any external tool that manipulates the file directly and omits empty/zero id attributes on serialization, or a first-ever sync of a local-only folder before Floccus has assigned it an id) — Floccus should tolerate that gracefully instead of poisoning its own id space with `NaN`.

### Expected Behavior
An item with a missing/invalid `@id` attribute should be assigned a fresh, valid, and **stable** id on parse (so it round-trips identically on the next sync), instead of `NaN`. It should never be possible for the failsafe to be triggered purely because of an id-parsing artifact rather than an actual bookmark change.

### To Reproduce
1. Point Floccus at a WebDAV account whose `bookmarks.xbel` contains a `` (or ``) with no `@id` attribute, e.g.:
```xml

NewProjectFolder
First dream

```
2. Sync — Floccus pulls this down into the local browser tree.
3. Add any bookmark locally inside that folder (or otherwise cause a local change under it) and sync again, so Floccus needs to write the tree back to the server.
4. Inspect the resulting `bookmarks.xbel` on the server: the folder now has `id="NaN"`, and after another sync cycle with a further local change, the same folder gets duplicated instead of updated.
5. With a large enough affected subtree, this trips: `Failsafe: The current sync run would delete N% of your local links in this profile. Refusing to execute.`

I have a debug log from the reproduction and a minimal fix ready — see PR #.

- [x] I will attach a debug log of the sync run that produced the error (available on request; reproduced in an isolated test environment, not my personal data)

---
If you like floccus, please consider [supporting it financially](https://floccus.org/download#donate) so it can be maintained long-term. Thank you!

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in src/lib/serializers/Xbel.ts at XbelSerializer._parseFolder and reproduce parsing a folder or bookmark without an @id. Trace how parsed ids are matched and serialized during the next sync. Done means missing or invalid ids become valid stable ids, never serialize as NaN, and the affected folder is not duplicated or falsely reported by the deletion failsafe.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
data
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.