Skip to content

Conversation

plexus
Copy link
Contributor

@plexus plexus commented Aug 31, 2021

Currently we are sending sideloaded resources as line-wrapped base64. The extra
newlines confuse the nREPL sideloader, turning the result into binary garbage.

I think in this case we should be both more conservative in what we send (this
patch) and more lenient in what we receive (nREPL should sanitize the input and
drop any characters that aren't valid base64).

Part of #3037


  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

Currently we are sending sideloaded resources as line-wrapped base64. The extra
newlines confuse the nREPL sideloader, turning the result into binary garbage.

I think in this case we should be both more conservative in what we send (this
patch) and more lenient in what we receive (nREPL should sanitize the input and
drop any characters that aren't valid base64).
@plexus
Copy link
Contributor Author

plexus commented Aug 31, 2021

The linter does not currently pass for me on master, but this PR does not introduce any new linting warnings. This PR does not add user visible functionality that needs to be mentioned in the manual.

@plexus
Copy link
Contributor Author

plexus commented Aug 31, 2021

Seems the linting does pass on CI. I installed eldev as per https://github.com/doublep/eldev#installation , and eldev lint gives me a bunch of warnings like

cider-doc.el:123:0 (checkdoc)    First sentence should end with punctuation
cider-doc.el:129:0 (checkdoc)    First sentence should end with punctuation
cider-doc.el:135:0 (checkdoc)    First sentence should end with punctuation
cider-doc.el:141:0 (checkdoc)    First sentence should end with punctuation
cider-doc.el:191:0 (checkdoc)    First line is not a complete sentence
Found 5 warnings in file ‘cider-doc.el’

cider-popup.el:30:0 (checkdoc)   First sentence should end with punctuation
Found 1 warning in file ‘cider-popup.el’

cider-scratch.el:74:0 (checkdoc) Lisp symbol ‘clojure-mode’ should appear in quotes
Found 1 warning in file ‘cider-scratch.el’

cider-stacktrace.el:89:0 (checkdoc) First sentence should end with punctuation
cider-stacktrace.el:95:0 (checkdoc) First sentence should end with punctuation
cider-stacktrace.el:101:0 (checkdoc) First sentence should end with punctuation
cider-stacktrace.el:107:0 (checkdoc) First sentence should end with punctuation
cider-stacktrace.el:113:0 (checkdoc) First sentence should end with punctuation
cider-stacktrace.el:119:0 (checkdoc) First sentence should end with punctuation
cider-stacktrace.el:125:0 (checkdoc) First sentence should end with punctuation
Found 7 warnings in file ‘cider-stacktrace.el’

is there some linter config I'm missing?

@bbatsov bbatsov merged commit 28fac9c into clojure-emacs:master Aug 31, 2021
@bbatsov
Copy link
Member

bbatsov commented Aug 31, 2021

Good catch, and good point about doing input sanitization on nREPL's side.

You can ignore the checkdoc warnings.

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