From notifications-return-52787-archive-asf-public=cust-asf.ponee.io@accumulo.apache.org Sat Nov 2 17:13:37 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 1F0E8180661 for ; Sat, 2 Nov 2019 18:13:37 +0100 (CET) Received: (qmail 25759 invoked by uid 500); 2 Nov 2019 17:13:36 -0000 Mailing-List: contact notifications-help@accumulo.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: jira@apache.org Delivered-To: mailing list notifications@accumulo.apache.org Received: (qmail 25745 invoked by uid 99); 2 Nov 2019 17:13:36 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 02 Nov 2019 17:13:36 +0000 From: GitBox To: notifications@accumulo.apache.org Subject: [GitHub] [accumulo] reggert opened a new pull request #1409: Corrected servlet parameter handling so that it does not break things Message-ID: <157271481598.32542.2413235326200706264.gitbox@gitbox.apache.org> Date: Sat, 02 Nov 2019 17:13:35 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit reggert opened a new pull request #1409: Corrected servlet parameter handling so that it does not break things URL: https://github.com/apache/accumulo/pull/1409 This resolves #1401. I removed the previous implementation of string sanitization, replacing it with more specific checks at the places where the parameters are used. As it turns out, the broken method was only used in two places, `ListType` and `ShowTrace`. In the former case, sanitization was only needed when including in the HTML output, so I used `StringEscapeUtils.escapeHtml` for that. In the latter case, sanitization was needed prior to passing the string in an Accumulo range, so I used a regular expression in `getTraceID` to validate that the string contains exactly 16 hexadecimal digits, which is how trace IDs are represented in the trace table. `BasicTest` was deleted because it was exclusively testing the broken sanitization mechanism. No other tests exist for the servlets, which is a problem but out of scope for what this patch is fixing. As a more long term solution, the servlets really ought to be replaced by JSP or another templating mechanism that can automatically handle escaping HTML content, rather that building HTML by concatenating Java strings. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: users@infra.apache.org With regards, Apache Git Services