Repository navigation
Autolink codeql warning - #512
Closed
ChrisJohnNOAA wants to merge 6 commits into
Closed
ChrisJohnNOAA wants to merge 6 commits into
ChrisJohnNOAA wants to merge 6 commits into
Conversation
* Replace custom URL detection with autolink-java library This change replaces the manual regex-based URL detection in String2.java with the autolink-java library (0.12.0) for improved accuracy and robustness. Key changes: - Added org.nibor.autolink:autolink dependency. - Created LinkHelper.java to encapsulate link extraction logic and support ERDDAP's legacy backslash-based protocols (e.g., http:\\\\). - Refactored String2.findUrl, containsUrl, and extractUrls to delegate to LinkHelper. - Consolidated calling patterns in Erddap.java, TableWriterHtmlTable.java, and Table.java using LinkHelper.splitByLinks to avoid redundant detection passes. - Updated String2Tests.java to reflect more accurate URL boundaries. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Replace custom URL detection with autolink-java library This change replaces the manual regex-based URL detection in String2.java with the autolink-java library (0.12.0) for improved accuracy and robustness. Key changes: - Added org.nibor.autolink:autolink dependency. - Created LinkHelper.java to encapsulate link extraction logic and support ERDDAP's legacy backslash-based protocols (e.g., http:\\\\). - Refactored String2.findUrl, containsUrl, and extractUrls to delegate to LinkHelper. - Consolidated calling patterns in Erddap.java, TableWriterHtmlTable.java, and Table.java using LinkHelper.splitByLinks to avoid redundant detection passes. - Updated String2Tests.java and TableTests.java to reflect improved link boundary detection and consistent HTML encoding. - Formatted affected files to comply with project standards. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Refactor URL detection with autolink-java and consolidate patterns This commit completes the transition from custom regex-based URL detection in String2.java to the autolink-java library (0.12.0). Key improvements: - Encapsulated logic in LinkHelper.java with a new lambda-style `linkify` method, significantly reducing code duplication in Erddap.java, TableWriterHtmlTable.java, and Table.java. - Enhanced detection to support ERDDAP's legacy backslash protocols (e.g., http:\\\\) and URLs containing quoted spaces (or %22). - Fixed a regression in Table.java where non-URL cell content was not being properly HTML-encoded. - Updated String2Tests.java and TableTests.java to align with improved, more accurate URL boundary detection. - Applied project-wide code formatting to resolve CI lint failures. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Finalized URL detection refactor with autolink-java This commit completes the transition from custom regex-based URL detection in String2.java to the autolink-java library (0.12.0), addressing all PR feedback and CI regressions. Key improvements: - Optimized LinkHelper.java to minimize redundant link extraction passes. - Enhanced NormalizedCharSequence to handle masking of spaces and quotes (including %22) within URLs, ensuring accurate boundary detection for complex query strings. - Consolidated link processing across Erddap.java, TableWriterHtmlTable.java, and Table.java using a new efficient lambda-based `LinkHelper.linkify` method. - Fixed HTML encoding logic in Table.java to prevent visual regressions in non-URL cell content. - Updated test suites (String2Tests, TableTests) to align with improved detection rules. - Resolved project-wide formatting and style issues. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Finalized URL detection refactor with autolink-java This commit completes the transition from custom regex-based URL detection in String2.java to the autolink-java library (0.12.0). Key changes: - Optimized LinkHelper.java with a single-pass `linkify` lambda method. - Supported quoted spaces and %22 in URLs via masking. - Consolidated link processing across Erddap.java, TableWriterHtmlTable.java, and Table.java. - Fixed HTML encoding regression in Table.java. - Updated String2Tests.java and TableTests.java to reflect improved accuracy. - Formatted affected files to comply with project standards. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Finalized URL detection refactor with autolink-java This commit completes the transition from custom regex-based URL detection in String2.java to the autolink-java library (0.12.0), incorporating all PR feedback and resolving regressions. Key improvements: - Optimized LinkHelper.java to identify and process links in a single pass using a lambda-based `linkify` method. - Enhanced detection to correctly handle URLs containing quoted spaces and %22, which is common in scientific data queries. - Consolidated calling patterns in Erddap.java, TableWriterHtmlTable.java, and Table.java, significantly reducing code duplication and redundant string splits. - Fixed an HTML encoding regression in Table.java to ensure consistent cell output. - Updated String2Tests.java and TableTests.java to reflect improved accuracy in URL boundary detection. - Applied project-wide formatting to ensure compliance with CI style checks. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Refactor URL detection to use autolink-java library - Add org.nibor.autolink:autolink:0.12.0 to pom.xml. - Create com.cohort.util.LinkHelper to encapsulate optimized URL detection. - LinkHelper uses NormalizedCharSequence to support backslash-based protocols and mask spaces/quotes within complex scientific query URLs. - Refactor String2.findUrl, containsUrl, and extractUrls to delegate to LinkHelper. - Consolidate URL linkification logic in Erddap.java, TableWriterHtmlTable.java, and Table.java using the new optimized LinkHelper.linkify method. - Update String2Tests.java to align with more accurate URL boundary detection. - Fix encoding regression in Table.java to ensure non-link text remains HTML-encoded. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Simplify LinkHelper API and remove redundant String2 wrappers - Remove findUrl, containsUrl, extractUrls, and addHttpsForWWW from String2.java. - Remove containsUrl from LinkHelper.java. - Add findUrl(String) overload to LinkHelper.java for improved usability. - Update String2Tests.java to use LinkHelper directly. - Ensure consistent code formatting. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Streamline LinkHelper API and refactor tests to use linkify - Remove findUrl methods from LinkHelper.java. - Rewrite String2Tests.java to use LinkHelper.linkify for URL detection verification. - Ensure all tests pass and code is correctly formatted. Co-authored-by: ChrisJohnNOAA <130599404+ChrisJohnNOAA@users.noreply.github.com> * Fix formatting --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
| # Set up the Java runtime specifically for the manual compilation step | ||
| - name: Set up JDK | ||
| if: matrix.build-mode == 'manual' | ||
| uses: actions/setup-java@v4 |
|
|
||
| # Initializes the CodeQL tools for scanning. | ||
| - name: Initialize CodeQL | ||
| uses: github/codeql-action/init@v4 |
| mvn clean compile -DskipTests | ||
|
|
||
| - name: Perform CodeQL Analysis | ||
| uses: github/codeql-action/analyze@v4 |
Comment on lines
+33
to
+36
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| # Set up the Java runtime specifically for the manual compilation step |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
I've started using CodeQL to highlight code concerns (it has a focus on security and performance issues). CodeQL highlighted the url extraction regexes as potentially extremely slow due to backtracking. While looking into fixes, I came across a library that does a better job than the regexes (there's some edge cases it handles correctly) and doesn't trigger CodeQL warnings.
Since I don't believe we have legitimate performance concerns with either approach I did not run a performance test.
Fixes CodeQL warnings. These aren't enabled on the main ERDDAP repo yet, and likely show up as security issues so I don't think I could link to them.
Type of change
Please delete options that are not relevant.
Checklist before requesting a review