search

getodk / briefcase

ODK Briefcase is a Java application for fetching and pushing forms and their contents. It helps make billions of data points from ODK portable. Contribute and make the world a better place! ✨💼✨
https://docs.getodk.org/briefcase-intro
Other
60 stars 156 forks source link

URI-encode spaces in URLs #881

Closed lognaturel closed 3 years ago

lognaturel commented 3 years ago

Closes #880

What has been done to verify that this works as intended?

Ran it with the form from the issue, wrote an automated test. I also pushed that same form and exported it to confirm there were no other issues with a form id with a space.

Why is this the best possible solution? Were any other approaches considered?

I initially thought I could use some kind of Java API for this but I don't think so. See https://stackoverflow.com/questions/4737841/urlencoder-not-able-to-translate-space-character. I decided to be very surgical and only handle spaces for now and we can deal with other characters that need to be URI-encoded if they actually come up.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The only thing this does is replace spaces in URIs with %20. It should be very safe.

Does this change require updates to documentation? If so, please file an issue at https://github.com/getodk/docs/issues/new and include the link below.

No.

kkrawczyk123 commented 3 years ago

@lognaturel pulling form with spaces in id doesn't work via cmd, error message Error: Form formid not found displays so only the first word in that kind of id is considered when using cmd. other actions that have been verified and work well for that kind of form:

lognaturel commented 3 years ago

pulling form with spaces in id doesn't work via cmd

This has more to do with how the command line works in general, I think. That is, spaces are meaningful separators so they must be escaped in some way if they're part of option text. On zsh, I can successfully pull the form from the issue by either escaping spaces with \ (formid\ with\ spaces) or wrapping the id in single quotes ('formid with spaces'). I think those are pretty standard. My sense is that this is fine.

kkrawczyk123 commented 3 years ago

Oh yes, I should think about that. I was able to pull, push and export using cmd right now. So we have success here! @getodk-bot unlabel "needs testing" @getodk-bot label "behavior verified"