search

remotestorage / remotestorage.js

⬡ JavaScript client library for integrating remoteStorage in apps
https://remotestoragejs.readthedocs.io
MIT License
2.32k stars 141 forks source link

Folder listings where ETags contains quotes are mishandled #1316

Open DougReeder opened 3 months ago

DougReeder commented 3 months ago

The standard says ETag values start and end with quotes (so weak ETags can be distinguished from strong). See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag

Per the RS standard folder listings must contain the ETag of documents. The example folder listings contain ETag values without the quotes, but the text doesn't clarify whether this is optional or required. As ETags are required to be strong, the quotes are not absolutely necessary.

When remotestorage.js is syncing, and a JSON folder listing contains ETags WITHOUT quotes (for example "ETag": "3f5cdfaa69bb4cdddaab87d28e5d918c",) remotestorage.js currently treats this as equal to the ETag returned by PUTs and GETs, even if they contain quotes.

When remotestorage.js is syncing, and a JSON folder listing contains ETags WITH quotes (for example "ETag": "\"3f5cdfaa69bb4cdddaab87d28e5d918c\"",) remotestorage.js currently treats this as unequal to the ETag value returned by PUTs and GETs if they include the quotes.

Given the ambiguity in draft-dejong-remotestorage-22.txt, remotestorage.js should parse ETags values in JSON folder listing the same, whether they include quotes or not.

raucao commented 3 months ago

I'm not sure this is how "quoted strings" in headers should be used in implementations. Isn't the idea of a quoted string, that only the characters between the quotes are the actual string/content?

DougReeder commented 3 months ago

Had I designed the HTTP spec, I would have done it differently. Stripping the quotes erases the difference between a strong and weak ETag.

To comply with the HTTP spec, the ETag values need to have quotes when used in a header. If the values in a folder listing can't have quotes, then code needs to either keep track of which kind of ETag a variable contains, or always add quotes to folder values, or always strip quotes from heading values. Each of these alternatives adds needless complication.

The one thing you can't do in the current situation is just use the same values everywhere.

raucao commented 3 months ago

According to RFC 9110, weak ETags must be prefixed with W/. And as you already pointed out, RS requires strong ETags, so there must never be weak ETags in folder listings. The examples in the spec also do not contain extra quotes.

So, I think this small inconsistency is OK to live with, because it feels more intuitive and less cluttered to not have extra quotes in the listings, which are then stripped by the client anyway.

That said, it wouldn't hurt much for rs.js to handle a server that decides to add extra quotes. However, it means that every client (not just rs.js) would then have to handle both cases, which seems like an unnecessary extra effort for non-rs.js clients to me.

Maybe adding a test for this to the server API test suite would be an agreeable alternative?

DougReeder commented 3 months ago

That (and documenting the requirement to strip quotes from folder listings) is probably the lesser effort, at this point.