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

Only validate header name once (attempt #2) #24499

Merged
merged 3 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -232,28 +232,28 @@ public void destroy() {

/*
* @see com.ibm.ws.genericbnf.internal.BNFHeadersImpl#findKey(byte[], int,
* int)
* int, boolean)
*/
@Override
protected HeaderKeys findKey(byte[] data, int offset, int length) {
return HttpHeaderKeys.find(data, offset, length);
protected HeaderKeys findKey(byte[] data, int offset, int length, boolean returnNullForInvalidName) {
return HttpHeaderKeys.find(data, offset, length, returnNullForInvalidName);
}

/*
* see com.ibm.ws.genericbnf.impl.BNFHeadersImpl#findKey(byte[])
* see com.ibm.ws.genericbnf.impl.BNFHeadersImpl#findKey(byte[], boolean)
*/
@Override
protected HeaderKeys findKey(byte[] name) {
return HttpHeaderKeys.find(name, 0, name.length);
protected HeaderKeys findKey(byte[] name, boolean returnNullForInvalidName) {
return HttpHeaderKeys.find(name, 0, name.length, returnNullForInvalidName);
}

/*
* @see
* com.ibm.ws.genericbnf.internal.BNFHeadersImpl#findKey(java.lang.String)
* com.ibm.ws.genericbnf.internal.BNFHeadersImpl#findKey(java.lang.String, boolean)
*/
@Override
protected HeaderKeys findKey(String name) {
return HttpHeaderKeys.find(name);
protected HeaderKeys findKey(String name, boolean returnNullForInvalidName) {
return HttpHeaderKeys.find(name, returnNullForInvalidName);
}

/*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/*******************************************************************************
* Copyright (c) 2017 IBM Corporation and others.
* Copyright (c) 2017, 2023 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-2.0/
*
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
Expand Down Expand Up @@ -64,9 +64,9 @@ public byte[] generateTrailerValue(HeaderKeys hdr, HttpTrailers message) {
@Override
public byte[] generateTrailerValue(String hdr, HttpTrailers message) {

HeaderKeys key = HttpHeaderKeys.find(hdr);
HeaderKeys key = HttpHeaderKeys.find(hdr, true);

if (key != null && hdr.equals(_key)) {
if (key != null && key.equals(_key)) {
if (tc.isDebugEnabled()) {
Tr.debug(tc, "generateTrailerValue(String,HttpTrailers): hdr = " + hdr + ", value = " + _value);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2004, 2009 IBM Corporation and others.
* Copyright (c) 2004, 2023 IBM Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
Expand Down Expand Up @@ -92,7 +92,8 @@ public void setFactory(HttpObjectFactory fact) {
* @return boolean (true if exists)
*/
public boolean containsDeferredTrailer(String target) {
return containsDeferredTrailer(findKey(target));
HeaderKeys key = findKey(target, true);
return key != null && containsDeferredTrailer(key);
}

/**
Expand Down Expand Up @@ -172,7 +173,7 @@ public void setDeferredTrailer(String hdr, HttpTrailerGenerator htg) {
if (null == htg) {
throw new IllegalArgumentException("Null value generator");
}
this.knownTGs.put(findKey(hdr), htg);
this.knownTGs.put(findKey(hdr, false), htg);
}

/**
Expand All @@ -196,7 +197,10 @@ public void removeDeferredTrailer(String hdr) {
if (null == hdr) {
throw new IllegalArgumentException("Null header name");
}
this.knownTGs.remove(findKey(hdr));
HeaderKeys key = findKey(hdr, true);
if (key != null) {
this.knownTGs.remove(key);
}
}

/**
Expand Down Expand Up @@ -314,24 +318,24 @@ public void writeExternal(ObjectOutput s) throws IOException {
}

/**
* @see com.ibm.ws.genericbnf.internal.BNFHeadersImpl#findKey(byte[], int, int)
* @see com.ibm.ws.genericbnf.internal.BNFHeadersImpl#findKey(byte[], int, int, boolean)
*/
protected HeaderKeys findKey(byte[] data, int offset, int length) {
return HttpHeaderKeys.find(data, offset, length);
protected HeaderKeys findKey(byte[] data, int offset, int length, boolean returnNullForInvalidName) {
return HttpHeaderKeys.find(data, offset, length, returnNullForInvalidName);
}

/**
* see com.ibm.ws.genericbnf.impl.BNFHeadersImpl#findKey(byte[])
* see com.ibm.ws.genericbnf.impl.BNFHeadersImpl#findKey(byte[], boolean)
*/
protected HeaderKeys findKey(byte[] name) {
return HttpHeaderKeys.find(name, 0, name.length);
protected HeaderKeys findKey(byte[] name, boolean returnNullForInvalidName) {
return HttpHeaderKeys.find(name, 0, name.length, returnNullForInvalidName);
}

/**
* @see com.ibm.ws.genericbnf.internal.BNFHeadersImpl#findKey(java.lang.String)
*/
protected HeaderKeys findKey(String name) {
return HttpHeaderKeys.find(name);
protected HeaderKeys findKey(String name, boolean returnNullForInvalidName) {
return HttpHeaderKeys.find(name, returnNullForInvalidName);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ public void reset() {
public void setTrailer(String name, String value) {

HttpTrailers trailers = message.createTrailers();
HeaderKeys key = HttpHeaderKeys.find(name);
HeaderKeys key = HttpHeaderKeys.find(name, false);

if (trailers.containsDeferredTrailer(key)) {
trailers.removeDeferredTrailer(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;

import com.ibm.wsspi.genericbnf.BNFHeaders;
import com.ibm.ws.ffdc.FFDCFilter;
import com.ibm.wsspi.genericbnf.HeaderKeys;
import com.ibm.wsspi.genericbnf.KeyMatcher;

Expand Down Expand Up @@ -240,7 +240,7 @@ public class HttpHeaderKeys extends HeaderKeys {
*
* @param name
*/
public HttpHeaderKeys(String name) {
private HttpHeaderKeys(String name) {
super(name, generateNextOrdinal());
if (NEXT_ORDINAL.get() <= ORD_MAX) {

Expand Down Expand Up @@ -275,7 +275,7 @@ private HttpHeaderKeys(String name, boolean undefined) {
* @param shouldLog
* @param shouldFilter
*/
public HttpHeaderKeys(String name, boolean shouldLog, boolean shouldFilter) {
private HttpHeaderKeys(String name, boolean shouldLog, boolean shouldFilter) {
super(name, generateNextOrdinal());
super.setShouldLogValue(shouldLog);
super.setUseFilters(shouldFilter);
Expand Down Expand Up @@ -336,30 +336,30 @@ public static HttpHeaderKeys match(byte[] name, int offset, int length) {
*
* @param name
* @param offset
* - starting point in that input name
* - starting point in that input name
* @param length
* - length to use from that offset
* - length to use from that offset
* @param returnNullForInvalidName
* - return null instead of throw IllegalArgumentException for header name validation
* @return HttpHeaderKeys
* @throws NullPointerException
* if input name is null
* @throws IllegalArgumentException
* if the input name contains CR or LF chars
* if the input name contains invalid chars
*/
public static HttpHeaderKeys find(byte[] name, int offset, int length) {
public static HttpHeaderKeys find(byte[] name, int offset, int length, boolean returnNullForInvalidName) {
HttpHeaderKeys key = (HttpHeaderKeys) myMatcher.match(name, offset, length);
if (null == key) {
synchronized (HttpHeaderKeys.class) {
// protect against 2 threads getting here on the new value by
// testing again inside a sync block
key = (HttpHeaderKeys) myMatcher.match(name, offset, length);
if (null == key) {
String headerName = new String(name, offset, length);
// make sure the name is valid
for (int i = offset; i < length; i++) {
if (BNFHeaders.CR == name[i] || BNFHeaders.LF == name[i]) {
throw new IllegalArgumentException("Invalid CRLF in name: " + i);
}
if (validateHeaderName(headerName, returnNullForInvalidName)) {
key = new HttpHeaderKeys(headerName, true);
}
key = new HttpHeaderKeys(new String(name, offset, length), true);
}
} // end-sync

Expand All @@ -372,13 +372,15 @@ public static HttpHeaderKeys find(byte[] name, int offset, int length) {
* never been seen prior, then a new object is created by this call.
*
* @param name
* @param returnNullForInvalidName
* - return null instead of throw IllegalArgumentException for header name validation
* @return HttpHeaderKeys
* @throws NullPointerException
* if input name is null
* @throws IllegalArgumentException
* if the input name contains CR or LF chars
* if the input name contains invalid chars
*/
public static HttpHeaderKeys find(String name) {
public static HttpHeaderKeys find(String name, boolean returnNullForInvalidName) {
HttpHeaderKeys key = (HttpHeaderKeys) myMatcher.match(name, 0, name.length());
if (null == key) {
synchronized (HttpHeaderKeys.class) {
Expand All @@ -387,32 +389,55 @@ public static HttpHeaderKeys find(String name) {
key = (HttpHeaderKeys) myMatcher.match(name, 0, name.length());
if (null == key) {
// make sure the name is valid
for (int i = 0, size = name.length(); i < size; i++) {
char c = name.charAt(i);
if (BNFHeaders.CR == c || BNFHeaders.LF == c) {
throw new IllegalArgumentException("Invalid CRLF in name: " + i);
}
if (validateHeaderName(name, returnNullForInvalidName)) {
key = new HttpHeaderKeys(name, true);
}
key = new HttpHeaderKeys(name, true);
}
} // end-sync
}
return key;
}

/**
* Find the enumerated object matching the input name. If this name has
* never been seen prior, then a new object is created by this call.
/*
* A valid header name is "!" / "#" / "$" / "%" / "&" / "'" /
* "*" / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
*
* @param name
* @return HttpHeaderKeys
* @throws NullPointerException
* if input name is null
* @throws IllegalArgumentException
* if the input name contains CR or LF chars
* The information about valid chars in a header name comes from
* RCF 9110 section 5.6.2 tchars
* https://www.rfc-editor.org/rfc/rfc9110.html#section-5.6.2
*
* PH52074
*/
public static HttpHeaderKeys find(byte[] name) {
return find(name, 0, name.length);
private static boolean validateHeaderName(String name, boolean returnFalseForInvalidName) {
for (int i = 0, size = name.length(); i < size; i++) {
char c = name.charAt(i);
// if we found an error, throw the exception now
if (!isValidTchar(c)) {
if (returnFalseForInvalidName) {
return false;
}
IllegalArgumentException iae = new IllegalArgumentException("Header name contained an invalid character " + i);
FFDCFilter.processException(iae, HttpHeaderKeys.class.getName() + ".validateHeaderName(String)", "1", name);
throw iae;
}
}
return true;
}

public static boolean isValidTchar(char c) {
boolean valid = ((c >= 'a') && (c <= 'z')) ||
((c >= 'A') && (c <= 'Z')) ||
((c >= '0') && (c <= '9')) ||
(c == '!') || (c == '#') ||
(c == '$') || (c == '%') ||
(c == '&') || (c == '\'') ||
(c == '*') || (c == '+') ||
(c == '-') || (c == '.') ||
(c == '^') || (c == '_') ||
(c == '`') || (c == '|') ||
(c == '~');

return valid;
}

/** private headers defined as sensitive */
Expand Down
Loading