Skip to content

dispatch on the request path only in the request uri dispatchers - #1318

Closed
kali834x wants to merge 2 commits into
apache:masterfrom
kali834x:request-uri-dispatch-path-only
Closed

kali834x wants to merge 2 commits into
apache:masterfrom
kali834x:request-uri-dispatch-path-only

Conversation

@kali834x

Copy link
Copy Markdown
Contributor

the request uri dispatchers resolve the service and operation through Utils.getServiceAndOperationPart, getOperationName and parseRequestURLForServiceAndOperation, which searched the whole To address for the last occurrence of the service context path and cut the query string only afterwards. AxisServlet and AxisHttpService put the query string into the To address for REST requests, so GET /axis2/services/Echo?next=/axis2/services/Admin/deleteAll ran Admin/deleteAll, and POST /axis2/services/Echo/axis2/services/Admin reached Admin through the repeated suffix, while the container, a servlet filter or a reverse proxy matched its URL patterns against /services/Echo, which is where SECURITY.md says endpoint authentication lives. strip the query string before the search in all three helpers and take the first occurrence of the service context path for the service; the operation keeps the right-to-left search for the service name, since a context root can legitimately repeat it earlier in the path. the kernel dispatcher tests and UtilsParseRequestTest gain cases that fail before the change, and a webapp integration test shows that GET /services/NoSuchService?next=/axis2/services/Version/getVersion returned the Version response before and does not after. the kernel, transport/http and webapp-tests suites pass.

@robertlazarski

Copy link
Copy Markdown
Contributor

Thanks, this is a good catch and the query-string fix looks right. Two spots still follow the same pattern, so it would be great to close them in this PR: getOperationName still uses
lastIndexOf(serviceName + "/"), so POST /axis2/services/Echo/echo/Echo/deleteAll still reaches deleteAll (searching for the operation only after the service-path index should fix it), and
HTTPLocationBasedDispatcher.parseRequestURL has its own lastIndexOf over the full To address including the query, so GET /axis2/services/Svc/safe?x=/Svc/delete resolves /delete. A test
for each, and a status-code check in RequestURIDispatchITCase, and I think this is ready.

@kali834x

Copy link
Copy Markdown
Contributor Author

Both fixed in ceb5c09. The operation dispatcher now reads the operation from the part after the service context path and takes the first occurrence of the service name, so /services/Echo/echo/Echo/deleteAll resolves echo. HTTPLocationBasedDispatcher only searches the path for the service name and the start of the location; the query stays in the location since location templates can match on it, but /Svc/safe?x=/Svc/delete now resolves /safe. Added a test for each (HTTPLocationBasedDispatcherTest is new), and RequestURIDispatchITCase checks 200 on the control request and 404 on both crafted ones. Kernel, transport/http and webapp-tests pass.

@robertlazarski

Copy link
Copy Markdown
Contributor

Merged to master as fc1b69c and acfcec2 (rebased onto master, so GitHub didn't detect the merge). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants