search

fnxweb / urllink

URL Link Firefox and Thunderbird add-on
https://addons.mozilla.org/en-GB/firefox/addon/url-link/
8 stars 0 forks source link

Prevent UTF-8 encoding of spaces within URL #7

Closed Eagle3386 closed 6 years ago

Eagle3386 commented 6 years ago

As shown in background.js, line 474, file:/// links get encoded under certain circumstances but keeps several characters from being encoded.

Recently, I had a Windows 7 machine which failed to access a Windows server 2012 UNC share, because whitespaces are encoded to %20. Without, i. e. replacing it with the original whitespaces again, it worked.

Hence, I'd like to ask for whitespaces being added to line 427.

fnxweb commented 6 years ago

Oh. file:/// URLs shouldn't be going into that encoder at all (ref. the comment). That's a bug — got an example?

Eagle3386 commented 6 years ago

Sure, there you go: Desired: file://///some-server/path/to.an/important folder/in some/deep/sub_directory/structure/

Received: file://///some-server/path/to.an/important%20folder/in%20some/deep/sub_directory/structure/

fnxweb commented 6 years ago

Ah, the ol' five-slash file: URL rears it's ugly head again ...

Eagle3386 commented 6 years ago

Thanks so much again, Neil!

fnxweb commented 6 years ago

So long as it actually works! 😃

Eagle3386 commented 6 years ago

Will test after vacation/upon add-on update.. 😉

Eagle3386 commented 6 years ago

@fnxweb Unfortunately, it doesn't work. In fact, URL Link seems to make it even worse now: \\some-server\path\to.an\important folder\in some\deep\sub_directory\structure\ gets turned into http://www.%5c%5csome-server%5cpath%5cto.an%5cimportant%20folder%5cin%20some%5cdeep%5csub_directory%5cstructure/

Notice that not only spaces remain encoded, but slashes get encoded, too (though, the trailing slash isn't) and the wrong prefix is added (http://www.).

fnxweb commented 6 years ago

Sorry, not ignoring you, my mail for this address was broken and I didn't notice!

fnxweb commented 6 years ago

OK, that's interesting, that URL worked fine for me when I tested it. I'll have another look.

fnxweb commented 6 years ago

OK, I'll be uploading 3.3.2 shortly, and that'll hopefully fix it. AMO doesn't support betas any more, and it's PITA to self-sign add-ons, so we may have to go round the houses here a couple of times (unless you have the dev. version installed which accepts unsigned add-ons)!

A wholly different bug to the one I fixed earlier; TBH, looking at the code, I can't figure out how it ever worked!

Eagle3386 commented 6 years ago

Regarding your comments:

  1. Didn't thought so since your reply on AMO a couple of weeks (months?) ago. So, don't mention it. :smiley:
  2. Thanks for taking another look. I'm happily helping your investigation, just drop me a line.
  3. I'm running Nightly here at home and Dev Edition at work - so, I just need a link to the XPI file. Though, common sites (like Dropbox, GDrive, etc.) are blacklisted at work. Maybe GitHub is sufficient?

:rofl: @ your last sentence. I don't know either.. :wink:

fnxweb commented 6 years ago

Cool, if it's still bollox I'll attach an XPI to this. Or you can always fetch it and add as a temporary plugin.

Eagle3386 commented 6 years ago

Sorry for being late, I've not recovered from conjunctivitis and a deep-seated cold. Used the short brake from sleep and all, to improve a couple of minor bugs (see my PRs) and opened #11.

Regarding this one, I think I'll try the latter - less work and faster results for you. :smiley:

Additionally, running version 3.3.2, the abovementioned \\some-server\path\to.an\important folder\in some\deep\sub_directory\structure\ now gets turned into file://///some-server/path/to.an/important folder/in some/deep/sub_directory/structure/ hence neither wrong prefix nor falsely encoded slashes.

So, I'd call this finally fixed. Once again, thanks for your awesome work, Neil! :+1:

fnxweb commented 6 years ago

Cool. I'll look at the other stuff over the weekend at some point.