Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for hashes in filenames, better uri encoding #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TBSliver
Copy link

Somehow having a hash in the filename, or something like that, caused the uri to 404 error. This fixes it, and actually encodes the outgoing uri properly. wooo!

@vanstyn
Copy link
Owner

vanstyn commented Jul 15, 2015

From IRC:

<vanstyn> TBSliver: unfortunately, as I feared, there are issues with your 
          Rapi::Fs PR, so I can't merge as-is...
<vanstyn> I'll update the PR itself with details when I have a free
          moment...
<TBSliver> yea no worries
<vanstyn> TBSliver: hey...
<vanstyn> so, since you're here, the main issue is that I wasn't to avoid 
          uri encoding across the board
<vanstyn> because its 1. ugly and 2. can cause issues with current tab 
          detection, since both forms still work
<vanstyn> I *think* that '#' is a special case...
<vanstyn> might also be '?' and '&'
<vanstyn> but, in general, we'd like to keep the url path matching the 
          file path as much as possible
<vanstyn> since, as you already saw, modern browsers already handle uri 
          encoding automatically for the most part
<vanstyn> but, I will do some testing and see what I can find out 
      (unfortunately i'm spread pretty thin atm)

@TBSliver
Copy link
Author

Have changed it to only percent encode specific characters ( these ones to be exact: &%#? ). more can be added quite quickly and easily in the same place as 99fbfd3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants