Fix issue 453 #487
Fix issue 453 #487
Conversation
markt-asf
force-pushed
the
fix-issue-453
branch
from
59ef6f5 to
b054725
Compare
|
Forgot to update the copyright. |
|
I think this is worth an explicit review because of the second commit. |
| @@ -212,6 +212,10 @@ public interface ServletContext { | |||
| * application) security constraints. Care should be taken both when constructing the path (e.g. avoid unsanitized user | |||
| * provided data) and when using the result not to create a security vulnerability in the application. | |||
| * | |||
| * <p> | |||
| * The provided {@code path} parameter is canonicalized as per section 3.5.2 of the specification before being used to | |||
There was a problem hiding this comment.
I don't really like this. As far as I can see this would actually set a new precedent, as none of the other javadoc we have explicitly references a section of the spec (I could be wrong, but it seemed like all other references were to RFC's rather than the Servlet spec).
If we are going to do this I think we need to explicitly specify the version of the spec, as at some point the section number will change and we may forget to update the javadoc.
There was a problem hiding this comment.
Turns out I was wrong, there is this:
This does not even specify which spec, let alone the version :-)
I really think that over time this will likely become outdated and rot, as these number can change if additional items are added, and it would be very easy to miss that they need to be updated, although I don't really have a good solution.
There was a problem hiding this comment.
We should probably come up with a very standard javadoc style for referencing a specific spec document and the section within it, so that we can easily find and update in future.
There was a problem hiding this comment.
How about this (with hyperlink to the right section in the HTML version of the spec):
Servlet 6.0, 3.5.2
We might need to tweak to working around the reference so that style works everywhere.
There was a problem hiding this comment.
Works for me as well.
markt-asf
force-pushed
the
fix-issue-453
branch
from
b054725 to
c8887cb
Compare
|
Servlet specification references updated to agreed format and PR rebased. |
This is intended to fix #453
The first commit should be uncontroversial.
I suspect the second commit will require some discussion but, logically, I don't currently see any viable alternative. My reasoning for this is explained in #453 (comment)