Skip to content

NIFI-15888 Add size limit to Standard Content Viewer#11303

Merged
pvillard31 merged 2 commits into
apache:mainfrom
exceptionfactory:NIFI-15888
Jun 2, 2026
Merged

NIFI-15888 Add size limit to Standard Content Viewer#11303
pvillard31 merged 2 commits into
apache:mainfrom
exceptionfactory:NIFI-15888

Conversation

@exceptionfactory
Copy link
Copy Markdown
Contributor

Summary

NIFI-15888 Adds an internal size limit of 10 MB to the Standard Content Viewer, avoiding attempts to load, format, and render large FlowFiles.

The internal limit of 10 MB aligns with a previous comment on Apache Avro formatting, which actually limited to the output size to 2 MB.

The implementation uses the LimitingInputStream to avoid reading more than 10 MB from the referenced FlowFile. The limit applies to both formatted and unformatted responses. Unformatted responses with content exceeding the limit return an HTTP 206 status indicating Partial Content returned. Formatted may throw an exception when the limit does not align with expected structural boundaries. In this case, the server returns an HTTP 500 with message indicating length exceeds maximum. As a best effort viewer, returning an error avoids both server and client memory consumption concerns. The hexadecimal viewer continues to provide a general way to view initial bytes.

Additional unit tests exercise length limiting behavior with formatted and unformatted responses.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
  • Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using ./mvnw clean install -P contrib-check
    • JDK 21
    • JDK 25

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

response.setStatus(HttpServletResponse.SC_OK);
}
} else {
contentStream.transferTo(response.getOutputStream());
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.

For content larger than the response buffer, the response commits during transferTo, so does the later setStatus(SC_PARTIAL_CONTENT) actually reach the client, or does it silently return 200 with truncated content?

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.

Good catch, the test behavior masked this important detail. I adjusted the approach to get the expected Content Length from the source, and used that to get and set the response status prior to handling the stream.

final LimitedStandardContentViewerController limitedController = new LimitedStandardContentViewerController();
limitedController.doGet(request, response);

verify(response).setStatus(eq(HttpServletResponse.SC_PARTIAL_CONTENT));
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.

Since MockServletOutputStream never commits the response, does this test really verify that a real container would send 206 after the body is written?

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.

Thanks for pointing this out, I reworked the implementation to set the response status earlier.

@exceptionfactory
Copy link
Copy Markdown
Contributor Author

Thanks for the review @pvillard31, good catch on the order of setting the response status.

I made some adjustments to get the source content length and pass it to the DownloadableContent object, enabling determination of partial content status prior to stream handling.

The LimitingInputStream still enforces the limit, even if the content length is something unknown, like -1, but the response status determination provides a best-effort approach to indicate whether truncation occurred.

Copy link
Copy Markdown
Contributor

@pvillard31 pvillard31 left a comment

Choose a reason for hiding this comment

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

Thanks @exceptionfactory - latest LGTM

@pvillard31 pvillard31 merged commit 28c0287 into apache:main Jun 2, 2026
14 of 16 checks passed
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.

2 participants