Optimized Logic to strip Malicious Header Characters like '\r' & '\n'.

[mc639]

Notes

Optimized Logic to strip Malicious Header Characters like '\r' & '\n'.

[carl-mastrangelo]

This would result in the input string being rebuilt each call to stripMaliciousHeaderChars. The previous implementation had a fast path for no invalid characters.

[mc639]

okay, so what I understand is that because I am not checking input.indexOf(c) != -1 the older implementation had a fast path.

[mc639]

Q: Do we want to remove \r\n when they are in pairs or also remove them individually?

From my research on the internet, we are stripping malicious header chars to prevent HTTP_Response_Splitting.

Here are my approaches to solve the above problem :
1.

    public static String stripMaliciousHeaderChars(@Nullable String input) {
        StringBuilder strippedInput = new StringBuilder();

        if (input == null) {
            return null;
        }

        if(input.indexOf('\r') != -1 && input.indexOf('\n') != -1) {
            for (char inputChar : input.toCharArray()) {
                if (inputChar != '\r' && inputChar != '\n') {
                    strippedInput.append(inputChar);
                }
            }
        }

        return strippedInput.toString();
    }

    public static String stripMaliciousHeaderChars(@Nullable String input) {
        
        if(input.indexOf('\r') != -1 && input.indexOf('\n') != -1) {
            return input.replaceAll("(\r\n|\n)", "");
        }
        
        return input;
    }

[carl-mastrangelo]

For example 1, wouldn't it need to be ||, rather than &&?

input.toCharArray is going to allocate an extra String. In order to optimize the function, we would need to establish a baseline performance profile, and then run the same benchmark with the PR patched in. For these kinds of changes, a micro benchmark is best. You should start off by writing a benchmark (see some examples in src/jmh/java), and then patch in your change.

For 2, it would not be acceptably fast to use a regex. That will allocate a new Pattern each time. Strings are not that bad to allocate, but building a Pattern is a lot worse.

Generally speaking, improving performance is a time-consuming process that involves a lot of reasoning about how the data looks in real life. If you are interested in contributing, I would suggest looking for an easier TODO. (perhaps for some bug or feature request)

[mc639]

makes sense, I surely want to start contributing and start learning from you and other helpful community members, I will take your advice and start with some easier TODO, do you have any TODO or bug in mind ??