search

readium / r2-streamer-js

NodeJS Readium2 "streamer"
BSD 3-Clause "New" or "Revised" License
21 stars 10 forks source link

Collision between keyParams and Path64 in NYPL Reader demo #27

Closed JayPanoz closed 6 years ago

JayPanoz commented 6 years ago

FYI we just had a bug with this one:

https://github.com/edrlab/r2-streamer-js/blob/b66d92e2c2a29ca31d252661deee9a0edeabc62f/misc/readers/reader-NYPL/index.html#L25-L39

as the streamer created a path64 path containing the "=" character and consequently split the path to the manifest.json.

So instead of 

https://website/pub/L3Zhci9qZWxseWJvb2tzL3JlYWRpdW0vYXNzZXRzL0ZhckZyb21UaGVNYWRkaW5nQ3Jvd2QtY292ZXJBLmVwdWI=/manifest.json

it returned

https://website/pub/L3Zhci9qZWxseWJvb2tzL3JlYWRpdW0vYXNzZXRzL0ZhckZyb21UaGVNYWRkaW5nQ3Jvd2QtY292ZXJBLmVwdWI
danielweck commented 6 years ago

Mhmm, based on your feedback I am not sure what the bug is though.

URL must of course be encoded appropriately when passed as query string parameters into other URLs, especially with conflicting Base64 reserved characters.

See for example how EPUB demo URLs are encoded in the r2-streamer-js basic online test interface:

https://readium2.herokuapp.com

(try using the childrens-literature.epub and wasteland-otf-obf.epub reader links)

danielweck commented 6 years ago

PS: this is in fact a very common problem which we had to address in the Readium ("1") cloud/web reader.

For example Accessible EPUB3 which is "shipped" with the deployed instance of the cloud reader: https://readium.firebaseapp.com/?epub=epub_content%2Faccessible_epub_3 (note the %2F slash character)

...or even childrens-literature.epub hosted at the r2-streamer-js Heroku server:

https://readium.firebaseapp.com/?epub=https%3A%2F%2Freadium2.herokuapp.com%2Fpub%2FL2FwcC9taXNjL2VwdWJzL2NoaWxkcmVucy1saXRlcmF0dXJlLmVwdWI%3D

JayPanoz commented 6 years ago

OK so PEKBAC indeed.

Sorry for the inconvenience.

Long debugging short: the first screen with all publications has "=" aplenty e.g. 

./pub/L1VzZXJzL0hlcnZlL3IyLXN0cmVhbWVyLWpzL21pc2MvZXB1YnMvTW9ieURpY2suZXB1Yg==

On the second screen i.e. publication specific:

?url=http%3A%2F%2Fwebsite%2Fpub%2FL1VzZXJzL0hlcnZlL3IyLXN0cmVhbWVyLWpzL21pc2MvZXB1YnMvTW9ieURpY2suZXB1Yg%3D%3D%2Fmanifest.json

Which is actually OK but for some reason we spent like 2 hours debugging that. Not that we’ve been silently assuming the first one could be used as is but we completely missed it. :-( Blame it on fatigue.