Skip to content
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

Fix 4535 - support file uploads with filenames containing single quote #4538

Merged
merged 1 commit into from Nov 4, 2022

Conversation

BrianDevC
Copy link
Contributor

@BrianDevC BrianDevC commented Oct 27, 2022

Description

Fix #4535

Single quotes in folder or file names were causing corruption of the shell command. The fix changes the ' character to '"'"'
For example a folder called te'st would now get shellQuoted to 'te'"'"'st'

  1. ' ends the first quote,
  2. " starts second quote,
  3. ' is valid character in double quotes
  4. " ends the second quote
  5. ' starts the third quote
  6. The end result when fed into the shell, is as we desired: te'st

#4535

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@shawkins
Copy link
Contributor

shawkins commented Oct 27, 2022

Looks good. It's also possible to just change \\\\ to be \\. Can you add/modify a test to PodIT.uploadFile? - there's one there already with a bunch of special chars, but not a single quote.

@BrianDevC
Copy link
Contributor Author

Looks good. It's also possible to just change \\ to be \. Can you add/modify a test to PodIT.uploadFile? - there's one there already with a bunch of special chars, but not a single quote.

I tried that but didn't get the results I expected.
Now trying it again it works just fine. Perhaps my sanity is leaving me. I'll re-patch to that. Adding a new assertion to the test you suggested.

CHANGELOG.md Outdated Show resolved Hide resolved
@BrianDevC
Copy link
Contributor Author

The code smells look to be A, not caused by my changes and B, fairly simple to fix. (Change throwable to exception.)

I'm happy to go ahead and commit that assuming tests still pass to keep it happy but am I alright to do so under this issue? Don't know your policy on lumping technically different issues together under the same number. :)

@shawkins
Copy link
Contributor

I'm happy to go ahead and commit that assuming tests still pass to keep it happy but am I alright to do so under this issue?

Sonar isn't perfect so it can find stuff that is out of scope or false positives. It won't hold up this pr to have code check failing, @manusa will merge it regardless.

If you'd like to address these, I don't have a problem with that.

Don't know your policy on lumping technically different issues together under the same number. :)

As long as they are this straight-forward, it's fine to have them under the same PR. As separate commits would be best.

@manusa manusa changed the title iss-4535 Fix 4535 - support file uploads with single quote filenames Nov 4, 2022
@manusa manusa changed the title Fix 4535 - support file uploads with single quote filenames Fix 4535 - support file uploads with filenames containing single quote Nov 4, 2022
Single quotes in folder or file names were causing corruption of the shell command.
The fix changes the ' character to '"'"'
For example a folder called te'st would now get shellQuoted to 'te'"'"'st'
1. ' ends the first quote,
2. " starts second quote,
3. ' is valid character in double quotes
4. " ends the second quote
5. ' starts the third quote
6. The end result when fed into the shell, is as we desired: te'st

fabric8io#4535

iss-4535

Changed to be much shorter as pointed out, manual still passes tests.
New test added
Changelog moved.

iss-4535

Fixed test brackets that mvn spotless:apply unfortunately didn't.

fabric8io#4535

iss-4535

Altered some catch statements to catch exceptions rather than throwables for sonar

fabric8io#4535
@manusa manusa added this to the 6.3.0 milestone Nov 4, 2022
@sonarcloud
Copy link

sonarcloud bot commented Nov 4, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

9.1% 9.1% Coverage
0.0% 0.0% Duplication

@manusa manusa merged commit 12cd4ca into fabric8io:master Nov 4, 2022
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.

File upload unusual behaviour on a ' character
3 participants