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 password and other astring tokens #56

Merged
merged 4 commits into from
Nov 2, 2017

Conversation

rhn
Copy link
Contributor

@rhn rhn commented Nov 2, 2017

Follow-up to @gcollura's pull request #20 , which seems somewhat stalled.

Differences:

  • username and most of mailbox params are quoted
  • added tests for quote!() itself

@rhn rhn mentioned this pull request Nov 2, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 62.963% when pulling 73f726b on rhn:quote-password into 542ee15 on mattnenterprise:master.

src/client.rs Outdated
@@ -601,7 +607,7 @@ mod tests {
let response = b"a1 OK Logged in\r\n".to_vec();
let username = "username";
let password = "password";
let command = format!("a1 LOGIN {} {}\r\n", username, password);
let command = format!("a1 LOGIN {} {}\r\n", quote!(username), quote!(password));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure what happened to the indentation here?

@jonhoo
Copy link
Collaborator

jonhoo commented Nov 2, 2017

Looks good to me! See the one inline comment about a weird indent; once that's fixed, I'd be happy to merge.

@rhn
Copy link
Contributor Author

rhn commented Nov 2, 2017

Thanks for noticing the indent - I removed some more tabs I previously missed, should be good now.

@jonhoo
Copy link
Collaborator

jonhoo commented Nov 2, 2017

Hmm, still there as far as I can tell? Are you sure you pushed to the right branch?
01ba3f9#diff-31bbf71c54bca98a0ae3d40a327af940R610

Some notable exceptions: messagebox names are defined as `INBOX / astring`, but with this change, INBOX is also quoted.
The `list-mailbox` token is not quoted, as it is not `astring`.
@rhn
Copy link
Contributor Author

rhn commented Nov 2, 2017

Rebasing is hard. I added a commit fixing indentation to be done with tabs.

@jonhoo
Copy link
Collaborator

jonhoo commented Nov 2, 2017

Haha, okay, thanks!

@jonhoo jonhoo merged commit d9caecc into mattnenterprise:master Nov 2, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 62.963% when pulling 01ba3f9 on rhn:quote-password into 542ee15 on mattnenterprise:master.

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 this pull request may close these issues.

None yet

4 participants