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

source list - running with -r fails to escape pipe (|) char #1614

Closed
jborean93 opened this issue Jul 16, 2018 · 10 comments
Closed

source list - running with -r fails to escape pipe (|) char #1614

jborean93 opened this issue Jul 16, 2018 · 10 comments
Assignees
Milestone

Comments

@jborean93
Copy link

What You Are Seeing?

When using choco source list -r, values the already contain a pipe | character are not escaped making it impossible to reliably parse the output programmatically. An example output where the source name and username contain a pipe is

test|pipe|https://chocolatey.org/api/v2/|False|user|pipe||0|False|False|False

When you compare it to the default API

Chocolatey|https://chocolatey.org/api/v2/|False|||0|False|False|False

You can see there are 2 more "columns" because both the name and user column contain the pipe char.

What is Expected?

Not sure, usual psv practices are to either quote all the values or just quote the values that contain the delimiter char, e.g.

"test|pipe"|https://chocolatey.org/api/v2/|False|"user|pipe"||0|False|False|False

How Did You Get This To Happen? (Steps to Reproduce)

Create the source with

choco source add --name "test|pipe" --source https://chocolatey.org/api/v2/ --user "user|pipe" --password password

List the sources with

choco source list -r

and you get

test|pipe|https://chocolatey.org/api/v2/|False|user|pipe||0|False|False|False

@ferventcoder
Copy link
Member

@jborean93 interesting - that's definitely unexpected. Something we should look to fix.

@ferventcoder ferventcoder changed the title choco source list -r fails to escape pipe (|) char source list - running with -r fails to escape pipe (|) char Sep 26, 2018
@ferventcoder
Copy link
Member

This has been completed for Chocolatey v0.10.12.

ferventcoder added a commit to ferventcoder/choco that referenced this issue Mar 11, 2019
In areas where we return pipe-delimited strings, it would be beneficial
to quote text where a pipe could be used in the string itself.
@ferventcoder
Copy link
Member

This was implemented on stable at 7f15fb0

@jborean93
Copy link
Author

@ferventcoder thanks for the update. I haven't tested to see if it's possible but wouldn't the changes in 7f15fb0 mean that if the field already has a double quote the parsing would now be broken? There is a .net class you can potentially use called TextFieldParser but it looks like it's not in .NET core which may be an issue in the future.

@ferventcoder
Copy link
Member

@jborean93 how would the field have a double quote? Perhaps there is a bit more we should do here. I could see an apostrophe, but a quote in the middle of the username would be really weird. However, so would a pipe, and here we are. Thoughts?

@jborean93
Copy link
Author

jborean93 commented Mar 11, 2019

Oh looking at the commit it is only quoting the username field but the repo name itself can also contain pipe characters. I'm just trying to think of a way that makes this generic to anything creating a PSV and not just a field within source.

However as for why the username would contain a quote, I would have no idea. People seem to want to do really weird things and being defensive about it is probably best. I did try to add a source with the " character but that just broke the argument parsing so someone would have to manually edit the XML to add this character. So in that sense it's probably not required, just thinking of future proofing.

ferventcoder added a commit that referenced this issue Mar 12, 2019
* stable:
  (GH-1562) Fix: --version includes warnings/errors
  (GH-1724) search - exit 1 on no results
  (GH-1614) Fix: quote if pipe found in string
ferventcoder added a commit that referenced this issue Mar 13, 2019
Also quote the source name if it includes a pipe character.
@ferventcoder
Copy link
Member

@jborean93 added for names as well.

ferventcoder added a commit that referenced this issue Mar 13, 2019
* stable:
  (maint) Corrected whitespace
  (GH-1689) Delete packaging scripts before upgrade
  (doc) fix grammar in scripting guidelines
  (doc) add don't use nupkg to scripting guidelines
  (maint) fix typo
  (GH-1602) exit 2 on items outdated
  (doc) add step for rebasing prior to merging
  (GH-1614) Quote source name if includes pipe
@jborean93
Copy link
Author

jborean93 commented Mar 15, 2019

@ferventcoder I take back what I said about not being able to use ". We actually test out both | and " as the source name, as well as some other non alphanumeric chars in our win_chocolatey_source test because of this issue https://github.com/ansible/ansible/blob/devel/test/integration/targets/win_chocolatey_source/defaults/main.yml. We parse the XML directly because we couldn't parse the command output reliably and this makes sure we don't regress in our feature offering for the future.

This means that the simple quote the value with " if it contains | still won't work in these cases.

@ferventcoder
Copy link
Member

@jborean93 perhaps open a new issue on this?

@jborean93
Copy link
Author

Honestly I'll probably give up on the -r output in favour of #1252. Having the output in JSON is a lot easier to parse for many libraries and is built into .NET.

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

No branches or pull requests

3 participants