Skip to content

Add Brotli(br) content decoding support #2572

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.

Sign up for GitHub

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

Merged
SanojPunchihewa merged 2 commits into from
Jun 26, 2026
+31 −1
Merged

Add Brotli(br) content decoding support #2572

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4679c5
Add Brotli(br) content decoding support
SanojPunchihewa Jun 23, 2026
ee9d450
Fix review comments
SanojPunchihewa Jun 24, 2026
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
4 changes: 4 additions & 0 deletions modules/transports/core/nhttp/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@
<artifactId>log4j-api</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.wso2.orbit.org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</dependency>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
Expand Down
24 changes: 23 additions & 1 deletion ...http/src/main/java/org/apache/synapse/transport/passthru/util/DeferredMessageBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.axis2.transport.http.*;
import org.apache.axis2.util.JavaUtils;
import org.apache.axis2.util.MessageProcessorSelector;
import org.apache.commons.compress.compressors.brotli.BrotliCompressorInputStream;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.http.protocol.HTTP;
Expand All @@ -39,6 +40,7 @@

import java.io.IOException;
import java.io.InputStream;
import java.io.PushbackInputStream;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
Expand Down Expand Up @@ -102,7 +104,7 @@ public OMElement getDocument(MessageContext msgCtx, InputStream in) throws

String contentType = (String) msgCtx.getProperty(Constants.Configuration.CONTENT_TYPE);
String _contentType = getContentType(contentType, msgCtx);
in = HTTPTransportUtils.handleGZip(msgCtx, in);
in = handleContentEncoding(msgCtx, in);

AxisConfiguration configuration = msgCtx.getConfigurationContext().getAxisConfiguration();
Parameter useFallbackParameter =
Expand Down Expand Up @@ -306,4 +308,24 @@ public static String getContentType(String contentType, MessageContext msgContex
}
return type;
}

private static InputStream handleContentEncoding(MessageContext msgContext, InputStream in) throws IOException {
if (checkContentEncodingMatches(msgContext, "br")) {
PushbackInputStream pushbackInputStream = new PushbackInputStream(in);
int bytesRead = pushbackInputStream.read();
if (bytesRead != -1) {
pushbackInputStream.unread(bytesRead);
return new BrotliCompressorInputStream(pushbackInputStream);
}
} else if (checkContentEncodingMatches(msgContext, "gzip")) {
return HTTPTransportUtils.handleGZip(msgContext, in);
}
return in;

@chanikag chanikag Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is different from the old behaviour. Can this break existing usecases?

@SanojPunchihewa SanojPunchihewa Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

HTTPTransportUtils.handleGZip returns the original input stream unchanged when the Content-Encoding header does not match gzip encoding. Therefore, this change does not affect the existing behaviour

@chanikag chanikag Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude comment: "The original code unconditionally called HTTPTransportUtils.handleGZip(msgCtx, in), which checks the Content-Encoding header internally and handles both gzip and x-gzip (a legacy alias used by some servers). The new code calls handleGZip only
when checkContentEncodingMatches(msgContext, "gzip") is true. If a server sends Content-Encoding: x-gzip, the new check misses it and returns the compressed stream undecoded."

@SanojPunchihewa SanojPunchihewa Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Following is the current code which only checks gzip.

public static InputStream handleGZip(MessageContext msgContext, InputStream in)
            throws IOException {
        Map headers = (Map) msgContext.getProperty(MessageContext.TRANSPORT_HEADERS);

        if (headers != null) {
            if (HTTPConstants.COMPRESSION_GZIP
                    .equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING)) ||
                    HTTPConstants.COMPRESSION_GZIP.equals(headers.get(
                            HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE))) {
                PushbackInputStream pushbackInputStream = new PushbackInputStream(in);
                int bytesRead = pushbackInputStream.read();
                if (bytesRead != -1) {
                    pushbackInputStream.unread(bytesRead);
                    return new GZIPInputStream(pushbackInputStream);
                }
            }
        }
        return in;
    }

}

private static boolean checkContentEncodingMatches(MessageContext msgContext, String encoding) {
Map headers = (Map)msgContext.getProperty(MessageContext.TRANSPORT_HEADERS);
return headers != null && (encoding.equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING))
|| encoding.equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE)));
}
Comment on lines +324 to +328

@coderabbitai coderabbitai Bot Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Match the Content-Encoding value case-insensitively.

checkContentEncodingMatches compares the header value with encoding.equals(...), which is case-sensitive. HTTP content-coding tokens are case-insensitive, so values such as BR, Br, or GZIP would not match and the body would be passed through undecoded, leading to a build/parse failure downstream. Consider normalizing the header value (e.g., trim + equalsIgnoreCase) before comparison.

♻️ Proposed adjustment 
     private static boolean checkContentEncodingMatches(MessageContext msgContext, String encoding) {
         Map headers = (Map)msgContext.getProperty(MessageContext.TRANSPORT_HEADERS);
-        return headers != null && (encoding.equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING))
-                || encoding.equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE)));
+        if (headers == null) {
+            return false;
+        }
+        Object value = headers.get(HTTPConstants.HEADER_CONTENT_ENCODING);
+        if (value == null) {
+            value = headers.get(HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE);
+        }
+        return value instanceof String && encoding.equalsIgnoreCase(((String) value).trim());
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static boolean checkContentEncodingMatches(MessageContext msgContext, String encoding) {
Map headers = (Map)msgContext.getProperty(MessageContext.TRANSPORT_HEADERS);
return headers != null && (encoding.equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING))
|| encoding.equals(headers.get(HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE)));
}
private static boolean checkContentEncodingMatches(MessageContext msgContext, String encoding) {
Map headers = (Map)msgContext.getProperty(MessageContext.TRANSPORT_HEADERS);
if (headers == null) {
return false;
}
Object value = headers.get(HTTPConstants.HEADER_CONTENT_ENCODING);
if (value == null) {
value = headers.get(HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE);
}
return value instanceof String && encoding.equalsIgnoreCase(((String) value).trim());
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/passthru/util/DeferredMessageBuilder.java`
around lines 326 - 330, The checkContentEncodingMatches method performs
case-sensitive string comparisons on HTTP Content-Encoding header values, which
violates HTTP specifications where content-coding tokens are case-insensitive.
Modify the method to normalize both the encoding parameter and the header value
retrieved from headers.get(HTTPConstants.HEADER_CONTENT_ENCODING) and
headers.get(HTTPConstants.HEADER_CONTENT_ENCODING_LOWERCASE) to lowercase and
trim any whitespace before performing the equality comparisons, allowing values
like BR, Br, or GZIP to correctly match their lowercase equivalents.

@SanojPunchihewa SanojPunchihewa Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a behavioural change and may break existing flows.

@coderabbitai coderabbitai Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@SanojPunchihewa — The concern is understandable, but this particular direction of change is strictly additive. equalsIgnoreCase is a superset of equals: every value that matches today (e.g., br, gzip) will continue to match after the change. The only new matches would be mixed-case variants (BR, GZIP, Br, etc.) that currently fall through undecoded. No currently working flow would be broken because no currently matching value would stop matching.

Per RFC 7231 §3.1.2.2, content-coding token comparisons are specified as case-insensitive, so the current case-sensitive behavior is technically a spec deviation rather than an intentional design choice. That said, if you'd prefer to keep the scope of this PR minimal and address case normalization separately, that's also a reasonable call.

}
6 changes: 6 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,11 @@
<artifactId>swagger-parser</artifactId>
<version>${swagger.parser.v3.version}</version>
</dependency>
<dependency>
<groupId>org.wso2.orbit.org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
<version>${commons-compress.orbit.version}</version>
</dependency>
</dependencies>
</dependencyManagement>
<reporting>
Expand Down Expand Up @@ -1676,6 +1681,7 @@
<org.wso2.orbit.atlassian.oai.version>2.44.1.wso2v1</org.wso2.orbit.atlassian.oai.version>
<swagger.parser.v3.version>2.1.37.wso2v1</swagger.parser.v3.version>
<jaxen.version>2.0.0</jaxen.version>
<commons-compress.orbit.version>1.28.0.wso2v1</commons-compress.orbit.version>

@chanikag chanikag Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn;t this v2?

</properties>
<developers>
<!-- If you are a committer and your name is not listed here, please include/edit -->
Expand Down
Loading