Quoted passwords

[reinux]

Re #12, allows for double-quoted passwords to contain spaces. Escapes backslash and quotes with backslash prefix.

I think I've covered most of the potential backtracking problems with the tests, but please take a look to make sure I'm not missing anything obvious.

The BNF in the blurb is severely complicated by this feature. I'm not too sure whether we should attempt to specify <value> accurately (in which case I would definitely need assistance), or if we should just expect users to read the remarks underneath.

Also, could you please advise on the style and string API usage in netRcToBuilder? It feels like it could be tidier, and I'm not 100% sure I'm building the strings optimally.

Lastly, FParsec recommends against using the monadic form for performance reasons, but I wonder if it might be a bit easier to read than the combinators, i.e.:

quotedPassword = do
  _ <- P.string "\"\""
  p <- P.many chars
  _ <- P.string "\"\""
  return $ BC.pack p

Thanks!

[andreasabel]

Thanks for the PR!

I think maybe "double quotes" was misunderstood...

[andreasabel]

Should this be "multi-word password", like a string in double-quotes?
Now it is double double-quotes...

[andreasabel]

I thing this should be (P.string "\"" *> P.many chars <* P.string "\"").

[andreasabel]

Would it be possible to parametrize hlp by the parser (tok or password) rather than duplicating it into hlpPassword?

[reinux]

That makes so much more sense. I don't know why I thought it would have been double double quotes LOL

No wonder it seemed weirdly redundant.

Will revise tonight.

[reinux]

So I've come across a complication arising from quotes, discovered thanks to QuickCheck: since account is printed after the password, if the password opens with an unclosed double quote and the account contains a quote, the check fails, e.g.

default login abc password "abc account def"

This seems like an unrealistic/unlikely scenario for the most part, though I can see cases where someone tries to use the library as a pretty printer and botches the file... maybe.

The only solution I can think of is to have the password be printed last. Then the only issue would be if there's a double quote in the comments, and comments aren't being checked for in the QC test anyway.

I wonder if other implementations have come across this problem at all, or if there are even any other implementations that generate netrc files...

Though, amusingly, neovim's syntax highlighting exposes the same problem. It probably isn't a severe issue in practice.

[andreasabel]

Maybe a robust way to handle this problem is to change the parser for passwords so that if the password begins with " then it is considered a quoted password (and also needs to end with "). If one wants a password like "abc it can be given as "\"abc".
The printer would always use the quoted form.

Btw, I notice that CI is broken, I am fixing it in PR #14 on which you can then rebase your PR.

[reinux]

Is there a way to have QC show the output produced by the code under test, i.e. the netsh string?

It's a bit difficult to diagnose this particular problem with just the parsed algebra:

  QC
    roundtrip:    FAIL
      *** Failed! Falsified (after 4 tests):
      NetRc {nrHosts = [NetRcHost {nrhName = "", nrhLogin = "\v", nrhPassword = "eHC", nrhAccount = "%l1", nrhMacros = [NetRcMacDef {nrmName = "/|9", nrmBody = "#\n"},NetRcMacDef {nrmName = "$(", nrmBody = "Ed8\n"}]},NetRcHost {nrhName = "\\\EM", nrhLogin = "Q", nrhPassword = "\ACK", nrhAccount = "d", nrhMacros = []},NetRcHost {nrhName = "\r\GS\SOH", nrhLogin = "", nrhPassword = ",D\SUB", nrhAccount = "\DLE\CAN", nrhMacros = []}], nrMacros = []}
      Use --quickcheck-replay=366047 to reproduce.
      Use -p '/roundtrip/' to rerun this test only.

[andreasabel]

Maybe you have to temporarily modify the properties to print extra information.
I know that this is possible, but not exactly how atm, maybe this post gives some ideas: https://stackoverflow.com/questions/4772902/how-to-display-a-reason-of-a-failed-test-property-with-quickcheck

[reinux]

Quick update: Got that problem fixed, but a password with quotes and a quote in between, e.g. "abc"123" gets parsed with the quotes, which certainly wouldn't be the intention most of the time.

Haven't had too much time to work on this the past week, but I'm still on it!