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

Initial creation of paket.config produces malformed file #2003

Closed
jdpage opened this issue Nov 3, 2016 · 14 comments · Fixed by #2110
Closed

Initial creation of paket.config produces malformed file #2003

jdpage opened this issue Nov 3, 2016 · 14 comments · Fixed by #2110

Comments

@jdpage
Copy link

jdpage commented Nov 3, 2016

Description

Running paket config add-credentials with no existing paket.config file produces a malformed paket.config file (encoding in XML header does not match actual file encoding) which subsequently cannot be read in by paket. Seems to be related to #1954

Repro steps

  1. Delete (or otherwise remove) your paket.config file
  2. Use paket config add-credentials <somedomain> to add to the credential store
  3. Run some paket command that hits the NuGet server (e.g. paket update)

Environment: Windows 10 Pro, 64-bit, paket 3.26.3 (also seen in 3.23.2).

Expected behavior

Command run in step 2 creates valid paket.config file; command run in step 3 uses the credentials entered in step 2.

Actual behavior

Command run in step 2 writes out a UTF-8 encoded paket.config file, but the <?xml ... ?> declaration at the top specifies it to be UTF-16. Command in step 3 is unable to read the file and exits with an error.

Known workarounds

Edit the paket.config file after creation so that the <?xml ... ?> declaration specifies UTF-8, or have an existing valid paket.config file.

@forki
Copy link
Member

forki commented Nov 4, 2016

@yannisgu IIRC the config feature was your contribution. Are you up for taking a look at this? thx

@yannisgu
Copy link
Contributor

yannisgu commented Nov 4, 2016

@forki, no time right now - maybe @adamralph which possibly "introduced" the bug in #1954?

@adamralph
Copy link
Contributor

adamralph commented Nov 4, 2016

I did draw attention (twice) to the addition of the header, but no one seemed to care.

@forki
Copy link
Member

forki commented Nov 4, 2016

yeah I got, but I didn't see that it will result in issue ;-)

@adamralph
Copy link
Contributor

I guess what probably needs to happen is the addition of overloads for both normalizeXml and saveFile with an encoding parameter, so that saveConfigNode can explicitly dictate the encoding to both.

@adamralph
Copy link
Contributor

(If overloads are even a thing in F# - this is where my F# knowledge fails me and prohibits me from sending a no-brainer PR.) 😄

@adamralph
Copy link
Contributor

Or perhaps normalizeXml should be changed to omit the encoding from the header?

@forki
Copy link
Member

forki commented Nov 4, 2016

why is it specifying UTF-16?

@forki
Copy link
Member

forki commented Nov 4, 2016

what do you guys think about d1d06b7 ?

@adamralph
Copy link
Contributor

I guess that would work too

@jdpage
Copy link
Author

jdpage commented Nov 8, 2016

Doesn't System.Xml.XmlWriter handle formatting and XML declarations and formatting and stuff properly for you? Going through that is probably safer than just assuming the file encoding.

If nobody minds I can author a pull request that uses that API instead, which should head off any possible future encoding-related problems.

@forki
Copy link
Member

forki commented Nov 8, 2016

Sure you can try that.

Am 08.11.2016 2:39 vorm. schrieb "Jonathan David Page" <
notifications@github.com>:

Doesn't System.Xml.XmlWriter handle formatting and XML declarations and
formatting and stuff properly for you? Going through that is probably safer
than just assuming the file encoding.

If nobody minds I can author a pull request that uses that API instead,
which should head off any future encoding-related problems.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2003 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADgNI0r77lx7AfWs2BgBCruVMniHqRFks5q79LWgaJpZM4Ko3y2
.

@jbaehr
Copy link

jbaehr commented Dec 2, 2016

Apparently, d1d06b7 does not fix the problem in all cases. I've got a machine here where paket hasn't been ever used; running v3.30.2. When I first run paket config add-credentaials https://...... I still get the "UTF-16" processing instruction in the config file. Any later call to paket fails with "Unable to parse Paket configuration from packages.config." (Note: wrong file name here; shuold be "paket.config"). When I delete the file (letting the parent folder in place) and run paket config add-credentials ... again, I get the correct file and paket works as expected after wards.

jdpage pushed a commit to jdpage/Paket that referenced this issue Jan 17, 2017
... instead of doing XmlWriter -> string -> disk.
- This guarantees that the file encoding and the XML declaration are
  properly in sync.
- Adds a `saveNormalizedXml` function to Utils to facilitate this.
- Adds a test to UtilsSpecs for the new function.
- Updates ConfigFile.saveConfigNode to use the new function.
- Should fix fsprojects#2003 permanently.
- Removes the previous fix in commit d1d06b7
@jdpage
Copy link
Author

jdpage commented Jan 17, 2017

Finally got a chance to come back around and do the System.Xml.XmlWriter version.

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 a pull request may close this issue.

5 participants