- 
                Notifications
    
You must be signed in to change notification settings  - Fork 103
 
Ensure UTF-8 charset is used to avoid IllegalArgumentException: Null charset name #1245
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes issue #1241 by specifying UTF-8 encoding when reading log files instead of using platform-dependent encoding. This ensures consistent behavior across different operating systems and environments where the default platform encoding may vary.
- Replaces platform encoding (null) with explicit UTF-8 encoding specification
 - Improves cross-platform compatibility for log file reading
 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| logError("MavenInvocationException: " + e.getMessage(), e); | ||
| 
               | 
          ||
| String invokerLogContent = JavadocUtil.readFile(invokerLogFile, null /* platform encoding */); | ||
| String invokerLogContent = JavadocUtil.readFile(invokerLogFile, "UTF-8"); | 
    
      
    
      Copilot
AI
    
    
    
      Aug 19, 2025 
    
  
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using StandardCharsets.UTF_8 instead of the string literal "UTF-8" to avoid potential typos and improve type safety.
| String invokerLogContent = JavadocUtil.readFile(invokerLogFile, "UTF-8"); | |
| String invokerLogContent = JavadocUtil.readFile(invokerLogFile, StandardCharsets.UTF_8.name()); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree with the bot here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other places like that
| logError("MavenInvocationException: " + e.getMessage(), e); | ||
| 
               | 
          ||
| String invokerLogContent = JavadocUtil.readFile(invokerLogFile, null /* platform encoding */); | ||
| String invokerLogContent = JavadocUtil.readFile(invokerLogFile, "UTF-8"); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree with the bot here.
          
 looks flaky  | 
    
## Description Since last upgrade of `maven-javadoc-plugin` charset difinition is required. Ref: apache/maven-javadoc-plugin#1245 ## Checklist <!--- Please delete options that are not relevant. Boxes should be checked by reviewer. --> - [ ] Enable backports when necessary (fex. [for bug fixes](https://github.com/camunda/camunda/blob/main/CONTRIBUTING.md#backporting-changes) or [for CI changes](https://github.com/camunda/camunda/wiki/CI-&-Automation#when-to-backport-ci-changes)). ## Related issues closes #
## Description Since last upgrade of `maven-javadoc-plugin` charset difinition is required. Ref: apache/maven-javadoc-plugin#1245 ## Checklist <!--- Please delete options that are not relevant. Boxes should be checked by reviewer. --> - [ ] Enable backports when necessary (fex. [for bug fixes](https://github.com/camunda/camunda/blob/main/CONTRIBUTING.md#backporting-changes) or [for CI changes](https://github.com/camunda/camunda/wiki/CI-&-Automation#when-to-backport-ci-changes)). ## Related issues closes #
## Description Since last upgrade of `maven-javadoc-plugin` charset difinition is required. Ref: apache/maven-javadoc-plugin#1245 ## Checklist <!--- Please delete options that are not relevant. Boxes should be checked by reviewer. --> - [ ] Enable backports when necessary (fex. [for bug fixes](https://github.com/camunda/camunda/blob/main/CONTRIBUTING.md#backporting-changes) or [for CI changes](https://github.com/camunda/camunda/wiki/CI-&-Automation#when-to-backport-ci-changes)). ## Related issues closes #
## Description Since last upgrade of `maven-javadoc-plugin` charset difinition is required. Ref: apache/maven-javadoc-plugin#1245 ## Checklist <!--- Please delete options that are not relevant. Boxes should be checked by reviewer. --> - [ ] Enable backports when necessary (fex. [for bug fixes](https://github.com/camunda/camunda/blob/main/CONTRIBUTING.md#backporting-changes) or [for CI changes](https://github.com/camunda/camunda/wiki/CI-&-Automation#when-to-backport-ci-changes)). ## Related issues closes #
## Description After reading [this issue ](apache/maven-javadoc-plugin#1245) thoroughly I understood that the actual fix was provided on javadoc 3.12.0 version ## Checklist <!--- Please delete options that are not relevant. Boxes should be checked by reviewer. --> - [ ] Enable backports when necessary (fex. [for bug fixes](https://github.com/camunda/camunda/blob/main/CONTRIBUTING.md#backporting-changes) or [for CI changes](https://github.com/camunda/camunda/wiki/CI-&-Automation#when-to-backport-ci-changes)). ## Related issues closes #
## Description After reading [this issue ](apache/maven-javadoc-plugin#1245) thoroughly I understood that the actual fix was provided on javadoc 3.12.0 version ## Checklist <!--- Please delete options that are not relevant. Boxes should be checked by reviewer. --> - [ ] Enable backports when necessary (fex. [for bug fixes](https://github.com/camunda/camunda/blob/main/CONTRIBUTING.md#backporting-changes) or [for CI changes](https://github.com/camunda/camunda/wiki/CI-&-Automation#when-to-backport-ci-changes)). ## Related issues closes #
@wendigo fixes #1241
On inspection I realized that the existing code is working at cross-purposes to the design of Java in that it catches an exception and converts it to null. That is, it converts an exception to an error flag that has to be explicitly checked for. Instead the exception should be caught and handled. I deprecated the method and replaced all usages within this plugin. Since the method is protected inside what should be (but isn't) a static utility class, I doubt anyone else is subclassing and invoking this, but just in case I left it in.