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

Dump valid JSON file and minor correction for opensshforwindows keys #354

Merged
merged 2 commits into from
Feb 19, 2019

Conversation

ocastejon
Copy link

Hi,

this PR addresses two issues that I detected when running LaZagne:

  • First, the JSON dump outputs an invalid JSON when special characters are in the result (for example, newline characters or quotes). In this case, if you try to load the content of the output file with json.loads (for instance) you will get an exception. This is what is described in issue JSON format stores multi-line string for SSH keys #226 regarding multi-line strings for SSH keys in the JSON output. This is due to the decode('unicode-escape') instruction when the JSON file is written:

     f.write(pretty_json.decode('unicode-escape').encode('UTF-8'))
    

    This will decode the special characters (\n for newline or " for quotes) that need to be encoded for the JSON to be valid according to its specification (see the answer to this stackoverflow thread). Substituting the previous line by:

    f.write(pretty_json.encode('UTF-8'))
    

    solves this issue.

  • Second, the password key for opensshforwindows credentials is spelled as PrivateKey (note the capital K). However, when writing the output, the code will convert the key to lowercase (see here). Later, the code will check that the password is not empty capitalizing only the first letter (see here). That is, it will look for the value stored under the key Privatekey which will not exist. Thus, the output will not contain the SSH key that was actually found. Changing the key from PrivateKey to Privatekey solves the issue.

I hope this is helpful. Please, let me know if you need any clarification or if you have any comment!

@AlessandroZ AlessandroZ merged commit e9f13f4 into AlessandroZ:master Feb 19, 2019
@AlessandroZ
Copy link
Owner

Thanks a lot for your PR, I appreciate 👍

@ocastejon
Copy link
Author

Wow that was quick! Glad to help!

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.

3 participants