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

x86_64: Update POST line ending for Quark #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

crogers1
Copy link

No description provided.

Quark server expects payload to end in two newlines.
Delimit all POST requests so that the server can parse
them properly.

Signed-off-by: Nicholas Tsirakis <[email protected]>
@jandryuk
Copy link
Contributor

Do you know if this breaks uid?

Would it make sense to move the CRLF addition into XUIXhr.post or XUIXhr.send so that individual callers don't need to be updated? There is only 1, so I guess it doesn't really matter.

The PR is weird since it's a merge commit and the actual commit. The change itself seems fine.

@crogers1
Copy link
Author

@jandryuk, Jed did an x64 build with Christopher's ocaml PRs and it looked ok, let me verify the line ending doesn't break anything. Also I'll drop the merge commit, I had Niko PR to my feature branch during development

@crogers1
Copy link
Author

crogers1 commented Mar 20, 2019

Tested on build 6451, UI log messages come through but syslog prints # 015 and # 012 instead of the line ending sequence, as they must not be escaped.

Functionally, no change, just the added junk at the end of the debug message.

@jandryuk
Copy link
Contributor

Great. Thanks for checking.

@crogers1
Copy link
Author

Merge commit dropped

Copy link
Contributor

@jandryuk jandryuk left a comment

Choose a reason for hiding this comment

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

LGTM

If someone cares, they could make uid strip off the trailing \r\n\r\n.

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.

2 participants