YARN-11941: Follow-up to YARN-11937: Improve the yarn.web-proxy.redirect-flag parameter detection in HTTP requests#8357
Open
roczei wants to merge 1 commit intoapache:trunkfrom
Open
YARN-11941: Follow-up to YARN-11937: Improve the yarn.web-proxy.redirect-flag parameter detection in HTTP requests#8357roczei wants to merge 1 commit intoapache:trunkfrom
roczei wants to merge 1 commit intoapache:trunkfrom
Conversation
…ect-flag parameter detection in HTTP requests Problem Statement The fix for YARN-11937 introduced a mechanism to handle yarn_knox_proxy=true (yarn.web-proxy.redirect-flag=yarn_knox_proxy) parameter in the HTTP query. However the current implementation might not correctly identify this parameter when it is part of a larger query string containing multiple parameters. For example in the following URL: ...?parameter1=true¶meter2=true&yarn_knox_proxy=true&doAs=user Currently the logic expects the query string to be exactly yarn_knox_proxy=true, it will fail to recognize the flag when other parameters (like doAs, parameter1, or parameter2) are present. https://github.com/apache/hadoop/blob/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/ hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/WebAppProxyServlet.java#L555 String redirectFlagName = conf.get(YarnConfiguration.PROXY_REDIRECT_FLAG, ""); if (!redirectFlagName.isBlank() && toFetch.getQuery().equals(redirectFlagName + "=true")) { ProxyUtils.sendRedirect(req, resp, toFetch.toString()); return; } Proposed Change: The parameter detection should be improved to properly parse the query string and identify the yarn_knox_proxy=true key-value pair regardless of its position or the presence of other parameters.
6 tasks
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Author
|
Found another bug. The issue isn't just with multiple parameters. If the toFetch.getQuery() doesn't have a path , it throws a java.lang.NullPointerException. I was able to reproduce it with this: Related line: WebAppProxyServlet.java:556 I'll fix it in this PR and update the Jira ticket too. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
Problem Statement
The fix for YARN-11937 introduced a mechanism to handle yarn_knox_proxy=true (yarn.web-proxy.redirect-flag=yarn_knox_proxy) parameter in the HTTP query. However the current implementation might not correctly identify this parameter when it is part of a larger query string containing multiple parameters. For example in the following URL:
Currently the logic expects the query string to be exactly yarn_knox_proxy=true, it will fail to recognize the flag when other parameters (like doAs, parameter1, or parameter2) are present.
https://github.com/apache/hadoop/blob/trunk/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/ hadoop-yarn-server-web-proxy/src/main/java/org/apache/hadoop/yarn/server/webproxy/WebAppProxyServlet.java#L555
Proposed Change:
The parameter detection should be improved to properly parse the query string and identify the yarn_knox_proxy=true
key-value pair regardless of its position or the presence of other parameters.
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
If an AI tool was used:
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html