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

Fix json's anger at newlines #415

Closed
wants to merge 1 commit into from

Conversation

@edunham
Copy link
Contributor

edunham commented Jun 30, 2016

Error without this fix:

credentials = json.loads('{...}')
      File "/usr/lib/python2.7/json/__init__.py", line 338, in loads
        return _default_decoder.decode(s)
      File "/usr/lib/python2.7/json/decoder.py", line 366, in decode
        obj, end = self.raw_decode(s, idx=_w(s, 0).end())
      File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
        obj, end = self.scan_once(s, idx)
    exceptions.ValueError: Invalid control character at: line 1 column 444 (char 443)

note: line 1 column 444 of the giant blob of secrets that I elided is the first occurrence of \n in the string.

Reason I think this works: Testing it in prod (sorry!) and also
http://stackoverflow.com/questions/9295439/python-json-loads-fails-with-valueerror-invalid-control-character-at-line-1-c


This change is Reviewable

@metajack
Copy link
Contributor

metajack commented Jun 30, 2016

Can we not fix the JSON?


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@edunham
Copy link
Contributor Author

edunham commented Jun 30, 2016

@metajack the newlines come in from pillar data which would get even less legible/editable if we removed them. So we could fix the json, but I'd prefer to keep the pillar pretty and just tell the parser not to break.

@metajack
Copy link
Contributor

metajack commented Jun 30, 2016

@bors-servo r+

Ok. Definitely seem the right thing to do then.

@bors-servo
Copy link
Contributor

bors-servo commented Jun 30, 2016

📌 Commit d3b5ca0 has been approved by metajack

@bors-servo
Copy link
Contributor

bors-servo commented Jun 30, 2016

Testing commit d3b5ca0 with merge 9732ffe...

bors-servo added a commit that referenced this pull request Jun 30, 2016
Fix json's anger at newlines

Error without this fix:

```
credentials = json.loads('{...}')
      File "/usr/lib/python2.7/json/__init__.py", line 338, in loads
        return _default_decoder.decode(s)
      File "/usr/lib/python2.7/json/decoder.py", line 366, in decode
        obj, end = self.raw_decode(s, idx=_w(s, 0).end())
      File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
        obj, end = self.scan_once(s, idx)
    exceptions.ValueError: Invalid control character at: line 1 column 444 (char 443)
```

note: line 1 column 444 of the giant blob of secrets that I elided is the first occurrence of `\n` in the string.

Reason I think this works: Testing it in prod (sorry!) and also
http://stackoverflow.com/questions/9295439/python-json-loads-fails-with-valueerror-invalid-control-character-at-line-1-c

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/415)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 1, 2016

💔 Test failed - travis

Error without this fix:

credentials = json.loads('{...}')
      File "/usr/lib/python2.7/json/__init__.py", line 338, in loads
        return _default_decoder.decode(s)
      File "/usr/lib/python2.7/json/decoder.py", line 366, in decode
        obj, end = self.raw_decode(s, idx=_w(s, 0).end())
      File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
        obj, end = self.scan_once(s, idx)
    exceptions.ValueError: Invalid control character at: line 1 column 444 (char 443)

Reason I think this works: Testing it and also
http://stackoverflow.com/questions/9295439/python-json-loads-fails-with-valueerror-invalid-control-character-at-line-1-c
@edunham edunham force-pushed the edunham:json-load-forgive-newlines branch from d3b5ca0 to a1ffb19 Jul 1, 2016
@edunham
Copy link
Contributor Author

edunham commented Jul 1, 2016

@bors-servo retry

@aneeshusa
Copy link
Member

aneeshusa commented Jul 1, 2016

How urgent is this fix? I think there is a better way to do this (i.e. doesn't insert the newlines) but it will take me a while to investigate.

@aneeshusa
Copy link
Member

aneeshusa commented Jul 1, 2016

The secrets causing the problem are minion-public-key and minion-private-key, both of which were removed in #222 and are no longer used. Removing them from the production pillar should fix the problem.

@edunham
Copy link
Contributor Author

edunham commented Jul 1, 2016

@aneeshusa oh awesome! I'll take them out and close this. Thank you!

@edunham edunham closed this Jul 1, 2016
@edunham edunham deleted the edunham:json-load-forgive-newlines branch Jul 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.