Skip to content

fix(rest): make visibility field case-insensitive in REST API#4150

Open
preet-p1981 wants to merge 1 commit into
eclipse-sw360:mainfrom
preet-p1981:fix/2569-case-insensitive-visibility
Open

fix(rest): make visibility field case-insensitive in REST API#4150
preet-p1981 wants to merge 1 commit into
eclipse-sw360:mainfrom
preet-p1981:fix/2569-case-insensitive-visibility

Conversation

@preet-p1981

Copy link
Copy Markdown

The convertToProject() method in ProjectController used String.toUpperCase() without an explicit Locale, and lacked validation against the Visibility enum. This caused two problems:

  • Locale-dependent uppercasing (Turkish 'i' bug).
  • Confusing failures when a client sent a value like 'private' or 'Private' instead of the canonical 'PRIVATE'.

Introduce a small static helper, ProjectController#parseVisibility(String), that:

  • trims whitespace,
  • uppercases using Locale.US,
  • validates against Visibility.values(),
  • throws IllegalArgumentException with the list of valid values when the input is unknown.

Add unit tests for the helper and two MockMvc regression tests that POST a project with lowercase / mixed-case visibility values.

Fixes #2569

Please provide a summary of your changes here.

  • Which issue is this pull request belonging to and how is it solving it? (Refer to issue here)
  • Did you add or update any new dependencies that are required for your change?

Issue:

Suggest Reviewer

You can suggest reviewers here with an @mention.

How To Test?

How should these changes be tested by the reviewer?
Have you implemented any additional tests?

Checklist

Must:

  • All related issues are referenced in commit messages and in PR

The convertToProject() method in ProjectController used String.toUpperCase() without an explicit Locale, and lacked validation against the Visibility enum. This caused two problems:

  * Locale-dependent uppercasing (Turkish 'i' bug).
  * Confusing failures when a client sent a value like 'private' or 'Private'
    instead of the canonical 'PRIVATE'.

Introduce a small static helper, ProjectController#parseVisibility(String),
that:
  * trims whitespace,
  * uppercases using Locale.US,
  * validates against Visibility.values(),
  * throws IllegalArgumentException with the list of valid values when the
    input is unknown.

Add unit tests for the helper and two MockMvc regression tests that POST a
project with lowercase / mixed-case visibility values.

Fixes eclipse-sw360#2569

Signed-off-by: preet-p1981 <preetpatelpjp@gmail.com>

@GMishx GMishx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor changes needed.

Also @preet-p1981 , please sign the ECA. Without it, we cannot accept changes from you. You can see, the status is failed in the CI checks.

return null;
}
try {
return Visibility.valueOf(value.trim().toUpperCase(Locale.US));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use Locale.ROOT instead.

* @return matching {@link Visibility} or {@code null} if input is null/blank
*/
static Visibility parseVisibility(String value) {
if (value == null || value.trim().isEmpty()) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please use CommonUtils.isNullEmptyOrWhitespace()

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.

fix(rest): Ensure visibility field is case-insensitive

3 participants