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

Option to turn off built in 'back' functionality in res.location('back') #3290

Open
WORMSS opened this issue Apr 27, 2017 · 4 comments
Open

Comments

@WORMSS
Copy link
Contributor

WORMSS commented Apr 27, 2017

I had spotted there were no tests for res.location('back') so I have added them.

I came across this because I was trying to navigate to the relative page called 'back' and was being redirected to domain root. It was not expected. After looking at the code I spotted there was a "special" case for the word "back".

@dougwilson
Copy link
Contributor

Hi @WORMSS as for the "It was not expected" comment, I felt like your documentation was pretty good (http://expressjs.com/en/4x/api.html#res.location). How can we improve it?

@WORMSS
Copy link
Contributor Author

WORMSS commented Apr 27, 2017

Oh, by "not expected" I mean I was expecting the exact result I put it to come out. After seeing the code and reading the docs, I understood why it was there.. But I was using "location" without realising there was special case for the word "back". If it had been something like :back or :referrer, something that is not a valid url safe, then I doubt I would have encountered it.

@dougwilson
Copy link
Contributor

Gotcha, so you're saying that the behavior was expected to treat back specially from what was in the documentation, you just think it could be done differently. You're not saying that it was undocumented and that there is any improvement we can make to the docs to make it clearer that using the string 'back' does not literally make the location 'back', right?

The reason I'm asking is because we can't change the behavior in 4.x, but I don't want people to not realize what is happening, so want to make sure that our documentation is sufficient such that it is not unexpected behavior.

@WORMSS
Copy link
Contributor Author

WORMSS commented Apr 27, 2017

As you have said in #3291 if you have the ability to remove the magic back code from 5.x, I would put my vote for that. You could keep the functionality, as I said above, maybe something that just isn't a common word, or something that is a valid referrer..
But no, your documentation on the use of the magic back is good.. I just hadn't read it before I stumped myself with it.

@WORMSS WORMSS changed the title Missing res.location('back') tests and option to turn off built in 'back' functionality Option to turn off built in 'back' functionality in res.location('back') Apr 27, 2017
WORMSS added a commit to WORMSS/express that referenced this issue May 11, 2017
Added tests for res.location('back')
Made option to turn off built 'back' referrer functionality when not
working the way it was expected.

fixes: expressjs#3290
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants