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

Content-type is being set to application/x-www-form-urlencoded #9

Closed
bbultman opened this issue Oct 5, 2016 · 5 comments
Closed

Content-type is being set to application/x-www-form-urlencoded #9

bbultman opened this issue Oct 5, 2016 · 5 comments

Comments

@bbultman
Copy link

bbultman commented Oct 5, 2016

Hi there!

Just found an issue using this package.

Lib: Splunk-logging v0.9.1
OS: MacOS 10.11.6
Node: 6.4.0
Splunk: 6.5.0

We're using a node script to send JSON data to splunk, and it's been working fine. Now I've stumbled upon an issue where the text includes parentheses inside a string.

For example, this:
id:329,build:(id:164979)
becomes:
id:329,build:%28id:164979%29
(as seen inside splunk)

I've isolated the issue to splunk-logging/splunklogger.js line 435, row 47.

The content-type is set to application/x-www-form-urlencoded, even though the default options for the request module include "json": true

I hope that line 435 can be removed entirely. It makes no sense to me -- both indirectly setting the content-type to json, and then explicitly setting it to x-www-form-urlencoded afterwards.

@shakeelmohamed
Copy link
Contributor

shakeelmohamed commented Oct 5, 2016

Hi @bbultman,

I believe we needed that header to workaround a bug in Splunk 6.3.
The right fix would be adding a config setting to determine if we want to send the application/x-www-form-urlencoded header, and default to not sending it.

Is this blocking you? If so, we're open to a pull request

@bbultman
Copy link
Author

bbultman commented Oct 6, 2016

It's somewhat urgent, but I need to wait for approval (company needs to sign the contribution agreement, etc). I'll be fixing this today (as per your recommendation), and if possible I'll create a PR for the fix in the future.

@shakeelmohamed
Copy link
Contributor

@bbultman That sounds like a plan. If needed, you can always fork and modify the library for a quick fix. I'll see about prioritizing the change internally

@shakeelmohamed
Copy link
Contributor

shakeelmohamed commented Oct 10, 2016

@bbultman after digging through this today, it looks like this URL encoding is "forced" upon us by the request library we use under the hood. I'm looking into our options.

edit: I've created a PR for request: request/request#2417

shakeelmohamed pushed a commit that referenced this issue Nov 8, 2016
For #6: catch errors when parsing the server response.
For #9: manually parse JSON responses, and omitting the urlencoded header.
@shakeelmohamed
Copy link
Contributor

We've just released v0.9.2 with this fix. Give it a try!

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

2 participants