Defaultrefererfix #976
Defaultrefererfix #976
Conversation
the url host an invalid referer was returned)
the referer host an invalid referer was returned
…present in" This reverts commit f41182d.
…resent in" This reverts commit fa7065c.
If the context path string was present in the referer host, the getReferer() method returned an invalid referer
On proxied app servers is common to have the context path suppressed from the external URL, although it is still valid within the app server context. If the request referer path does not start with the internal context, it should return the entire path.
directly; replacing several test methods for referer check with redirect/forward with a single method that tests the getReferer() returned value in various situations
|
that sounds nice to me, @valeriolopes! thanks for doing this. |
|
|
||
| when(request.getHeader("Referer")).thenReturn("/vrap/anything/ok/vrap"); //relative path in the referer (not common but predicted by http spec) | ||
| when(request.getContextPath()).thenReturn("/vrap"); | ||
| assertEquals("/anything/ok/vrap", refererResult.getReferer()); |
There was a problem hiding this comment.
split this test into 4 @Test's, with the appropriate descriptions as the method name instead of the comments.
method into four specific tests
|
Done as @lucascs asked, breaking down the test method into more specific tasks. Besides, added a more safe check on the referer path, because the previous condition doesn't cover all cases: return refererPath.startsWith(ctxPath) ? refererPath.substring(ctxPath.length()) : refererPath; If the context path strings appears in the beggining of the url path but it is not the context path (ex: context path = /vrap; url path = /vrapanything/test), this would return anything/test, a wrong path. It's a very specific condition but it can occur. In order to remove the context path from the url, it should appear in the beggining of the url path in one of those conditions: 1) being followed by a '/' and anything (or nothing) after it; 2) be equal to the url path (nothing should come after it).In this case, we return what's after the context path. This covers all cases: Context path = /ctx I thought using a regexp to test this - ^/ctx(/.*|$) would do it - but thought again because I would have to compile the regex every time the referer was requested since the context path is not a fixed value (well.. it's a fixed value after the web app is started but I would have to capture that info somewhere and save it to compile it once.. maybe too much work). Ended up staying with plain old string and its fast char array indexing. |
|
|
||
| @Test | ||
| public void whenRefererIsARelativePathRefererShouldBeReturnedCorrectly() throws Exception { | ||
| when(request.getHeader("Referer")).thenReturn("/vrap/anything/ok/vrap"); //relative path in the referer (not common but predicted by http spec) |
There was a problem hiding this comment.
you can drop the comments on all tests,
then 🚢
|
Done |
fixing DefaultRefererResult when context path appears in the URL host
|
thanks! |
|
YW -- It was an honor giving this tiny contribution to vraptor. If you guys need more hands to help I'll be around (maybe fixing bug reports or whatever else). Is there any technical documentation of the framework or the code is the only (and prob the best) document? |
|
that was a nice contribution... not tiny at all 👍
great! if you want to, you can check our issues
take a look on http://www.vraptor.org/en/docs/one-minute-guide/ |
|
From the framework user perspective I have a good knowledge; I meant something about its internals (how it is organized from inside), but since the code is well structured with some study one should learn about it. BTW, how do you guys organize these issues?You just pick up one issue, branch it, do whatever you want and then submit it for review? |
|
You can use our test coverage to get into the framework, it's a very effective |
Relaxing getReferer() visibility to 'protected' in order to unit test it
directly; replacing several test methods for referer check with
redirect/forward with a single method that tests the getReferer()
returned value in various situations