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

Escape paths with n #152

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Escape paths with n #152

wants to merge 2 commits into from

Conversation

res0nance
Copy link

@res0nance res0nance commented Jan 14, 2020

Signed-off-by: Raihaan Shouhell raihaan.shouhell@autodesk.com

Looks like escaping windows paths does not work with \n

CC: @alexanderrtaylor

Raihaan Shouhell added 2 commits January 14, 2020 14:33
Signed-off-by: Raihaan Shouhell <raihaan.shouhell@autodesk.com>
Signed-off-by: Raihaan Shouhell <raihaan.shouhell@autodesk.com>
@res0nance res0nance marked this pull request as ready for review January 15, 2020 02:10
Copy link

@alexanderrtaylor alexanderrtaylor left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this @res0nance and the splitting of the string seems good to me. Tests are better than the ones I originally designed to capture windows specific paths

Strange that it didnt work before...

@mdres
Copy link

mdres commented Feb 13, 2020

Tbh manually messing with the escaping at all seems like a bad idea. This PR definitely improves the situation though, because the weird re-escape behavior will trigger less often, but it will still actively screw up people that actually escape their paths correctly according to the Java Property format.

Since some people probably rely on the old behavior, maybe this PR can be improved further by trying to guess and exclude already escaped paths from the re-escaping (only touch things matching [A-z]:\\\\[^\\].* for example).

At least this behavior should be clearly documented. I just found this PR because I was wondering why our generated .property file with properly escaped umlauts (these things: ü, ö, ä) was injected incorrectly.

Property files can contain all kinds of things, \f, \t, \uXXXX, multi-line values, keys with escaped spaces and separators, all of which is handled by Property.load correctly if you play by the rules and correctly escape your inputs ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants