feat: Add Aviator source encoding handling - #1066
Open
kireetivar wants to merge 11 commits into
Open
Conversation
- Introduced SourceEncodingOptions to manage source encoding candidates for decoding and encoding source files. - Updated AuditFprOptions to include default source encoding options. - Modified AuditProcessor and RemediationProcessor to accept and utilize SourceEncodingOptions. - Enhanced FileUtils to read files using specified source encoding options. - Updated SourceCodeEnricher to decode source files based on encoding options. - Adjusted AviatorGrpcClient and AviatorStreamProcessor to pass source encoding options through gRPC calls. - Updated AviatorSSCApplyRemediationsCommand and AviatorSSCAuditCommand to accept source encoding parameters from the command line. - Added new properties for source encoding options in internationalization files for both Aviator and FoD.
kireetivar
changed the title
kireetivar
marked this pull request as ready for review
July 31, 2026 09:07
rsenden
requested changes
Jul 31, 2026
Contributor
There was a problem hiding this comment.
I haven't reviewed everything in detail, focusing on overall structure first. Instead of having custom option parsing functionality, it would be much better to use existing picocli functionality; something like the following:
- Create
ISourceDecoderinterface, with singledecodemethod matching signature of currentSourceEncodingOptions::decode - Create implementations of this interface:
- One implementation in which the
decodemethod retrieves encoding from FVDL - One that takes a fixed
Charsetas constructor arg (together with a String-based constructor that callsCharset.forName) - Optionally a
CompositeSourceDecoderthat takes aList<ISourceDecoder>as constructor argument
- One implementation in which the
- Create a corresponding picocli
ITypeConverterthat based on input (FPRor explicit charset) returns an instance of one of the classes above, throwing a proper picocli exception likeTypeConversionExceptionto have picocli render a proper error message in case of invalid charset - Create picocli mixin/arggroup class (ideally in
fcli-aviator-commonfor re-use across FoD/SSC, but not sure whether we currently have any picocli-related classes in this module) that defines--source-encodingsasList<ISourceDecoder>(or directly asCompositeSourceDecoder, but not sure whether that would work withsplit), with propersplit,converter, anddefaultValueattributes - Pass this list of candidate decoders to the processors (either as-is, or wrapped in
CompositeSourceDecoder, but for flexibility, processors would rely onISourceDecoderinterface) - For encoding, have a separate
SourceEncoderclass with staticencodemethod, as this operates on specifiedCharsetand doesn't (directly) rely on any of the above
…ecoder hierarchy
rsenden
approved these changes
Aug 3, 2026
…source file failure reporting
…and improve test coverage
rsenden
requested changes
Aug 19, 2026
| private final String displayMessage; | ||
| private final Pattern messagePattern; | ||
|
|
||
| AuditSkipReason(String messageFormat, String displayMessage, Pattern messagePattern) { |
Contributor
There was a problem hiding this comment.
Please consider Lombok @RequiredArgsConstructor
| return auditSkipReason == null ? AuditSkipReason.from(status, statusMessage) : auditSkipReason; | ||
| } | ||
|
|
||
| public AuditResponse(AuditResult auditResult, int inputToken, int outputToken, String status, |
Contributor
There was a problem hiding this comment.
This code was already present before this PR, but this constructor takes way too many arguments, thus easy to get the order wrong. Please consider (Lombok) builder pattern instead (for this or future PR).
| if (message.isBlank()) { | ||
| return UNKNOWN; | ||
| } | ||
| for (AuditSkipReason reason : values()) { |
Contributor
There was a problem hiding this comment.
This is still doing string-based matching; matching has just been updated from string comparison to pattern matching. It would be better to simply store the AuditSkipReason enum value in AuditResponse, such that you can directly access the appropriate enum entry and associated display message. That might also remove the need for having UNKNOWN and/or OTHER enum values.
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
3 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.
Summary
--source-encodings, defaulting toFPR,UTF-8,ISO-8859-1.