Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HBASE-22467 UI fixes to enable Knox proxying #261

Closed
wants to merge 2 commits into from

Conversation

joshelser
Copy link
Member

No description provided.

} else {
super.doGet(req, resp);
}
}

static String sanitize(String input) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular rationale for this approach rather than using the existing org.apache.hadoop.hbase.http.HtmlQuoting.quoteHtmlChars(String) method?

I guess the quoting approach doesn't protect against an attacker setting the Accept header to javascript at the same time they include a query parameter that would cause the browser to execute said javascript in the echoed page.

I'm surprised there isn't already a utility method for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope! Just ignorant of that class.

@@ -37,6 +38,8 @@
private static final long serialVersionUID = 1L;
private static final Logger LOG = LoggerFactory.getLogger(ProfileOutputServlet.class);
private static final int REFRESH_PERIOD = 2;
// Alphanumeric characters, plus percent (url-encoding), equals, and ampersand
private static final Pattern ALPHA_NUMERIC = Pattern.compile("[a-zA-Z0-9\\%\\=\\&]*");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:should also accept a literal + for spaces I think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree!

@joshelser
Copy link
Member Author

Pushed edaae03 which uses HtmlQuoting. Don't see a need to re-invent the wheel and I think the implementation of that method is fine.

HBaseClassTestRule.forClass(TestProfileOutputServlet.class);

@Test
public void testSanitization() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't these tests fail now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, yes.

@joshelser
Copy link
Member Author

Ripping out edaae03 -- doesn't work for what I'm trying to do here. Will leave a comment on commit.

asfgit pushed a commit that referenced this pull request May 28, 2019
Closes #261

Signed-off-by: Sean Busbey <busbey@apache.org>
asfgit pushed a commit that referenced this pull request May 28, 2019
Closes #261

Signed-off-by: Sean Busbey <busbey@apache.org>
asfgit pushed a commit that referenced this pull request May 28, 2019
Closes #261

Signed-off-by: Sean Busbey <busbey@apache.org>
@asfgit asfgit closed this in 858d30d May 28, 2019
@joshelser joshelser deleted the 22467-knox-ui-fixes-master branch May 28, 2019 20:49
symat pushed a commit to symat/hbase that referenced this pull request Feb 17, 2021
Closes apache#261

Signed-off-by: Sean Busbey <busbey@apache.org>
(cherry picked from commit dbcf286)

Change-Id: I4704a54d5933c61d412cfeb709a2284416283b94
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