Skip to content

HTML-encode the JSON response based on the content type#7385

Open
labkey-jeckels wants to merge 4 commits intodevelopfrom
fb_extJsonHtmlResponse
Open

HTML-encode the JSON response based on the content type#7385
labkey-jeckels wants to merge 4 commits intodevelopfrom
fb_extJsonHtmlResponse

Conversation

@labkey-jeckels
Copy link
Contributor

@labkey-jeckels labkey-jeckels commented Feb 3, 2026

Rationale

A certain combination of HTTP headers can cause mixup in terms of Content-Type and encoding. I don't think this will happen in real-world scenarios, but can be hit by security scanners.

The reported scenario is doing a POST to query-import without the X-Requested-With=XMLHttpRequest HTTP request header. The actual form submission sends that header, so the server responds with application/json. Without the header, the server returns JSON with as text/html but fails to HTML encode.

Changes

  • HTML encode the JSON, per the pre-existing comment

Tasks 📍

  • Manual Testing - N/A
  • Needs Automation - N/A

@labkey-jeckels labkey-jeckels self-assigned this Feb 3, 2026
@labkey-jeckels labkey-jeckels requested a review from a team February 4, 2026 17:25
@Override
public void writeProperty(String name, Object value) throws IOException
{
super.writeProperty(sendHtmlJsonResponse ? PageFlowUtil.filter(name) : name, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a comment here to the effect that super.writeProperty() calls writeObject() which encodes

}
else
{
super.writeObject(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

So all non-String values are safe to render without encoding?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In practice, yes. But there are other possible values that could end up rendering as strings. I was able to change the override approach to catch more of those theoretical pathways. I didn't see a way to intercept this line though:

        else if (isSerializeViaJacksonAnnotations() || value instanceof SimpleResponse<?>)

Copy link
Contributor

Choose a reason for hiding this comment

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

If everything is supposed to be encoded, could this be tackled from the stream side?

Copy link
Contributor

Choose a reason for hiding this comment

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

If everything is supposed to be encoded, could this be tackled from the stream side?

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.

3 participants