Vojtech Szocs has posted comments on this change.
Change subject: utils: ForwardServlet: fix generic forward to use path info
......................................................................
Patch Set 1:
(2 comments)
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/ForwardServlet.java
Line 91
Line 92
Line 93
Line 94
Line 95
This means "uri" can be null, so code in service method:
String forwardUri = uri;
if (request.getPathInfo() != null) {
forwardUri += request.getPathInfo();
}
RequestDispatcher dispatcher = forwardContext.getRequestDispatcher(forwardUri);
can cause "forwardUri" to be either null (if request.getPathInfo() == null), or
even worse, "null/extra/path/info" (if request.getPathInfo() != null).
Just like in original code, we should always ensure "uri" isn't null, so it
should be like this:
if (uri == null) {
throw new ServletException("Target URI not defined in web.xml");
//$NON-NLS-1$
}
This also implies "uri" init-param is now mandatory, i.e. without any fallback
value.
Line 113: ServletException {
Line 114: final ServletContext forwardContext =
getServletContext().getContext(targetContext);
Line 115: if (forwardContext != null) {
Line 116: String forwardUri = uri;
Line 117: if (request.getPathInfo() != null) {
Hm, this change has implications that should be documented in ForwardServlet's
Javadoc.
Please remove following comment from the Javadoc:
The uri can be '*' which means all URLs relative to the ForwardServlet context
are used to determine
the path in the target servlet context. So if the ForwardServlet is mapped to
/docs and the target context
is /ovirt-engine/docs. Then /docs/something gets forwarded to
/ovirt-engine/docs/something. <br />
I'd also suggest to include something like this in the Javadoc:
Any extra path associated with the URL (i.e. any path after the servlet
context path)
will be appended to given URI relative to the target context path.
Line 118: forwardUri += request.getPathInfo();
Line 119: }
Line 120: final RequestDispatcher dispatcher =
forwardContext.getRequestDispatcher(forwardUri);
Line 121: if (dispatcher != null) {
--
To view, visit http://gerrit.ovirt.org/20784
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7307d57792fb966c8acb6ac0bdcad9302c0e3a65
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
