Conversation
iwillig
force-pushed
the
ivan/upgrade-hiccup
branch
2 times, most recently
from
4a68733 to
f16375e
Compare
This change attempts to migrate clojars to the latest version of hiccup, which has introduced a bunch of new security features. In this change, we also remove the old safe_hiccup, as this functionality is built into Hiccup2. #914
iwillig
force-pushed
the
ivan/upgrade-hiccup
branch
from
f16375e to
f8a4331
Compare
tobias
left a comment
tobias
left a comment
There was a problem hiding this comment.
Thanks for this, it looks pretty good! There is some whitespace change that isn't needed; would you mind running ./bin/cljstyle fix in the root of the project and including those fixes in your commit? Thanks!
|
|
||
| (defn tag [s] | ||
| (raw (html [:span.tag s]))) | ||
| (raw (str (h/html [:span.tag s])))) |
There was a problem hiding this comment.
Will this value still be unescaped? You can confirm that by running the app locally and looking at the home page. The Maven Repository example should show xml.
There was a problem hiding this comment.
Will do, good point.
There was a problem hiding this comment.
I think this works ok too?
| (raw (str (h/html [:span.tag s])))) | |
| (h/html [:span.tag s])) |
| (set! *warn-on-reflection* true) | ||
|
|
||
| ;; Helper functions to replace safe-hiccup functionality | ||
| (defn raw |
There was a problem hiding this comment.
Do we need this? Or can we just call h/raw directly? (We already do in quite a few places in this PR).
There was a problem hiding this comment.
Yeah, Good point. happy to remove it.
Of course! |
danielcompton
left a comment
danielcompton
left a comment
There was a problem hiding this comment.
Nice upgrade. I think that it would work ok to make RawString Renderable. That would simplify the code and mean you don't need to wrap things in str as much. I'm open to other opinions though.
| (str title " - ")) | ||
| "Clojars"] | ||
| (map #(include-css (str "/stylesheets/" %)) | ||
| (str (html5 {:lang "en"} |
There was a problem hiding this comment.
Instead of wrapping these in str calls, what do you think about this?
(extend-protocol compojure.response/Renderable
RawString
(render [body request]
(compojure.response/render
(str body)
request)))
I guess the downside is that you could accidentally return a RawString unintentionally, but I'm not sure if that is a real issue.
|
|
||
| (defn tag [s] | ||
| (raw (html [:span.tag s]))) | ||
| (raw (str (h/html [:span.tag s])))) |
There was a problem hiding this comment.
I think this works ok too?
| (raw (str (h/html [:span.tag s])))) | |
| (h/html [:span.tag s])) |
| "<!DOCTYPE svg PUBLIC \"-//W3C//DTD SVG 1.1//EN\" | ||
| \"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd\">" | ||
| (svg-template (jar-name jar) (:version jar))))) | ||
| (str (h/html |
There was a problem hiding this comment.
This can be removed if you make RawStream renderable.
3 participants
Switch to hiccup2.0 in clojars code base
This change attempts to migrate clojars to the latest version of
hiccup, which has introduced a bunch of new security features.
In this change, we also remove the old safe_hiccup, as this
functionality is built into Hiccup2.
#914