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

wrap-out middleware should qualify symbols that rely on later runtime resolve calls #542

Closed
mrrodriguez opened this issue Aug 3, 2018 · 0 comments

Comments

@mrrodriguez
Copy link

mrrodriguez commented Aug 3, 2018

In cider.nrepl.middleware.out/print-stream
the proxy OutputStream for the PrintWriter later bound to System/setOut ends up doing this

(resolve '*out*)

(same goes for *err*)

The point is that this is an unqualified reference to *out*, instead of clojure.core/*out*. This can cause issues if System/out is being called from *ns*'s that do not automatically have clojure.core/*out* mapped to the ns-interns.

In particular, I saw this come up in a CLJS macroexpansion situation, where System/out was called and the *ns* was bound to cljs.main during the CLJS compiler's analysis stage.

I believe a solution to this issue may be to fully-qualify these symbols so that the macro doesn't implicitly rely on what *ns* is bound to when these PrintStream.write methods are called.

Expected behavior

wrap-out to work transparently when System/out called in application.

Actual behavior

wrap-out can sometimes cause NullPointerException if the caller calls System/out with a *ns* bound that cannot resolve the bare, unqualified symbol *out* or *err*.

This comes up in CLJS if there is a macro that calls something like (.println System/out s) during macroexansion with the *ns* bound to the cljs.main ns.

Steps to reproduce the problem

I believe all you need to show it is

(ns fail-wrap-out-middleware
  (:refer-clojure :exclude [*out*]))

(.println System/out "10")

A real-world example is vvvvalvalval/scope-capture#24 (comment)

Environment & Version information

cider-nrepl version

0.17.0

but this is still in master and is in older versions as well.

Java version

1.8, 1.9 etc

@bbatsov bbatsov closed this as completed in 54bef72 Aug 4, 2018
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

No branches or pull requests

1 participant