Follow GH example for component URI encoding on branch name #795
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Example: * https://github.com/Martii/UserScriptsTesting/branches displays `this&that` with a href of [`this/Martii/UserScriptsTesting/tree/this%26that`](https://github.com/Martii/UserScriptsTesting/tree/this%26that)... note that `&` does still currently work if not percent encoded but symmetry is what is being looked for here in our code base. It is notable that [`/Martii/UserScriptsTesting/tree/this&that`](https://github.com/Martii/UserScriptsTesting/tree/this&that) also works... but don't know for how long. Luckily a repo branch can't be made of `?` *(at least in git-gui)* so we wouldn't have that issue... but if it did at any point... we could have an issue.
Martii
added
enhancement
Martii
added a commit
that referenced
this pull request
Nov 3, 2015
Follow GH example for component URI encoding on branch name Auto-merge
Martii
pushed a commit
to Martii/OpenUserJS.org
that referenced
this pull request
Nov 17, 2015
* Some whitespace adjustment for STYLEGUIDE.md * Drop `slug` terminology except where currently needed... this still might be completely going away but smaller steps to avoid mass breakage and increase mutual understanding * Partial fix for an encoding component issue ... there are more TODO's needed to fix all of it * Some controller symmetry * Using some ...`URIComponent` as indicated in OpenUserJS#795 ... needs individual handling of username and scriptname and avoids some encoding/decoding issues **NOTES** * This is not done yet but getting closer * Should be a near parallel change with a partial bug fix Applies to OpenUserJS#819 , OpenUserJS#200, and Originally applies to OpenUserJS#262 ... treads on OpenUserJS#262 (comment) *(preauth)*
Martii
pushed a commit
to Martii/OpenUserJS.org
that referenced
this pull request
Feb 20, 2016
* Remove the patch I put in for this portion * Create function for encoding like GH's... see helper `encoding` * Using encodeURIComponent from OpenUserJS#795 * Fix bug of Fork History showing "slugged" instead of native/raw * Fix a missing model parser item for `category` virtual model * Fix some more bugs in modelParser.js * Transform a watchpoint with AuthAs ... see third alternative at http://stackoverflow.com/questions/15825333/how-to-reload-current-page-in-express-js with `aRes.redirect(req.get('referer'));` or *express*@4.x shortcut of `aRes.redirect('back');`... but if referer is missing will default to `/` Ref: http://expressjs.com/en/api.html#res.redirect ... this still appears to be a bug in *express* for quite some time so this is a workaround * Add another series of watchpoints with *express*@4.x `redirect()` doing `escape` on occasion ... special handling of `redirect`.... this particular bug doesn't appear to make much sense * Project version bump **NOTES** * Followup with DB migration to add `fork.utf` to existing forks otherwise they show up as `/` * Followup with `discussion.path` fixes. * If *express* fixes their redirect issue can simplify `encode` to be a smarter encode where it only encodes when needed... which is where it started but had to back that out in this PR * Leaving GH import alone as it still works so far * `./app.js` has one `encodeURI` in it... leaving that be for the time being until it can be retested on production... dev isn't https so no way to test here Applies to OpenUserJS#819 and OpenUserJS#262... treads on OpenUserJS#262 (comment) among many others.
Martii
pushed a commit
to Martii/OpenUserJS.org
that referenced
this pull request
Feb 21, 2016
* Remove legacy namespace in all routes ... part of OpenUserJS#262 and OpenUserJS#130... officially EOL as per sizzle and confirmed by me... no redirects. * Remove the patches I put in for this portion * Create function for encoding similar to GH's... see helper `encoding` * Using encodeURIComponent from OpenUserJS#795 * Fix bug of Fork History showing "slugged" instead of native/raw * Fix category reference bug... redirect was set to the Object itself and not one of its properties * Fix some more missing object identifiers in modelParser.js ... not all are validated yet and some may still be missing for future proofing * Remove and Add some more watchpoints * Flip successful "rating updated" for Groups to debug mode and related in `./libs/modelParser.js` ... these are generating too much log traffic OpenUserJS#430 **NOTES** * [ ] Followup with DB migration to add `fork.utf` to existing forks otherwise they may not show up next to the bullet * Leaving GH import alone as it still works so far ... however a watchpoint added... GH web UI always `encodeURIComponent` just about everything... will need to reaffirm existing `encodeURI` later * The term uri is inclusive to urls as previously described in OpenUserJS#819 ... however in this context url means it could be partially encoded whereas uri means it absolutely is encoded... except for the slashes... normally those are encoded but we don't want them encoded... again following GH example. A url containing `ä` may be just that however a uri has `%E4`. You will notice that I renamed some of our identifiers from `url` to `uri`... this is on purpose to indicate that is what we are expecting. * *express* `redirect()` uses [`Location`](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.30) and [`Content-Location`](http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.14) headers. This requires all urls to be at least URI encoded as per specs... e.g. all the browsers are adding in an `escape` for the section symbol *which may be an issue with them*. So hence why the naming of `url` vs `uri`. * The base issue of readability of the code is still there... but at least this punches out some of the bugs mentioned. * Compression/optimization hasn't occurred yet * No DB migration is necessary now... this includes fixing `Re:` linkage * The smarter encoder isn't finished yet as I may be rearranging this a bit but... **This should knock out the BLOCKING label I put on**... at least from my perspective. Applies to OpenUserJS#819 and OpenUserJS#262... treads on OpenUserJS#262 (comment) among many others.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Example:
this&thatwith a href ofthis/Martii/UserScriptsTesting/tree/this%26that... note that&does still currently work if not percent encoded but symmetry is what is being looked for here in our code base. It is notable that/Martii/UserScriptsTesting/tree/this&thatalso works... but don't know for how long. Luckily a repo branch can't be made of?(at least in git-gui) so we wouldn't have that issue... but if it did at any point... we could have an issue (without this).Loosely applies to #200
Additional note... it is unclear if GH's server is rewriting or the browser is rewriting the URI... so best to use "full" percent encoding at this time.