jneilliii / OctoPrint-PrusaSlicerThumbnails

91 stars 20 forks source link

Fix encoding on client side #90

Closed NotExpectedYet closed 2 years ago

NotExpectedYet commented 2 years ago

Noticed this whilst coding OctoFarm as I had the same issue when saving / requesting urls with weird and wonderful character sets.

Fixes the encoding on client side when pulling the URLs. Let me know if I've missed anything and I'll happily amend.

jneilliii commented 2 years ago

what happens when this is run against an already encoded file from the python side? the latest rc has this patched here.

NotExpectedYet commented 2 years ago

what happens when this is run against an already encoded file from the python side? the latest rc has this patched here.

Aye I noticed that after submitting and seeing the issue. Would that fix pre-existing files? my python is still spotty. Would make this pointless imo.

Nothing really, encodeURI only replaces special characters so existing filenames won't be touched by it.

Weird file name

url: http://10.50.1.175:5000/plugin/prusaslicerthumbnails/thumbnail/WEIRD%22£$$()£$£$)(.png?20220503013428
encodeURI output: http://10.50.1.175:5000/plugin/prusaslicerthumbnails/thumbnail/WEIRD%2522%C2%A3$$()%C2%A3$%C2%A3$)(.png?20220503013428

None weird file name:

url: http://10.50.1.175:5000/plugin/prusaslicerthumbnails/thumbnail/CORNER_BRACEx12.png?20220420033926
encoded: http://10.50.1.175:5000/plugin/prusaslicerthumbnails/thumbnail/CORNER_BRACEx12.png?20220420033926

For example in OF's file manager where this is applied with the current plugin state with a mix of weird and wonderful names and normal names.

image

jneilliii commented 2 years ago

yeah, broken links would have to be redone. I could fix that up with a version upgrade/settings migration hook though when I go to release next version.

Nothing really, encodeURI only replaces special characters so existing filenames won't be touched by it.

except if the rc had been out there the filenames would already be encoded so wouldn't the % character in an encoded url then get encoded again? I always get confused with the js/html side of these things...lol.

NotExpectedYet commented 2 years ago

yeah, broken links would have to be redone. I could fix that up with a version upgrade/settings migration hook though when I go to release next version.

Nothing really, encodeURI only replaces special characters so existing filenames won't be touched by it.

except if the rc had been out there the filenames would already be encoded so wouldn't the % character in an encoded url then get encoded again? I always get confused with the js/html side of these things...lol.

Sorry I get what you were meaning now, if it's already encoded in the back end, what would encodeURI do with a pre-encoded file name.

console.log(encodeURI("http://10.50.1.175:5000/plugin/prusaslicerthumbnails/thumbnail/WEIRD%2522%C2%A3$$()%C2%A3$%C2%A3$)(.png?20220503013428"))

output: http://10.50.1.175:5000/plugin/prusaslicerthumbnails/thumbnail/WEIRD%252522%25C2%25A3$$()%25C2%25A3$%25C2%25A3$)(.png?20220503013428

So yeah it get's re-encoded again. This wouldn't work very well!

Personally this is much simpler than re-factoring the backend but that's entirely up to you aha. I'm also not privy to the changes in 1.8.0 so I'll have to check that myself for OctoFarm's sake.

NotExpectedYet commented 2 years ago

I'll run a few tests on 1.8.0-RC5 and come back to you

NotExpectedYet commented 2 years ago

Good spot!

Latest OP and PST RC's break this implementation completely. Will close as it's not needed.