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

Remove sanitization #117

Merged
merged 3 commits into from
Sep 20, 2014
Merged

Remove sanitization #117

merged 3 commits into from
Sep 20, 2014

Conversation

rtfb
Copy link
Collaborator

@rtfb rtfb commented Sep 19, 2014

Following discussion in #90, this seems to be the minimal first cut at what needs to be done to transfer sanitization duties to bluemonday: removed what we had in blackfriday and added a reference to the docs.

@dmitshur
Copy link
Collaborator

I'll review this thoroughly this evening, but huge 👍 to the overall direction! Thanks!

/cc @buro9


// ...
unsafe := blackfriday.MarkdownCommon(input)
html := bluemonday.UGCPolicy().Sanitize(string(unsafe))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to rewrite this line as:

html := bluemonday.UGCPolicy().SanitizeBytes(unsafe)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly, fixed that. Thanks!

@dmitshur
Copy link
Collaborator

Reviewed with more attention, and aside from comment above, this LGTM.

@buro9
Copy link
Contributor

buro9 commented Sep 20, 2014

Also reviewed it, cannot see anything lacking. Go for it.

@rtfb
Copy link
Collaborator Author

rtfb commented Sep 20, 2014

Thanks for eyeballing.

rtfb added a commit that referenced this pull request Sep 20, 2014
@rtfb rtfb merged commit 64fbfbb into russross:master Sep 20, 2014
@rtfb rtfb deleted the remove-sanitization branch September 21, 2014 12:36
@tianon
Copy link

tianon commented Jan 27, 2016

It appears that this was the last place within blackfriday that was still referencing the now-dead code.google.com/p/go.net/html -- any chance we could get a new release tag to match so the latest release version builds properly without patches? 😄 (cc @russross)

See moby/moby#19787 for a little context.

@tianon
Copy link

tianon commented Jan 27, 2016

Wow, I'm special -- this PR was included in both 1.3 and 1.4; sorry for the noise! Hopefully I can track down the correct PR without too much trouble...

@tianon
Copy link

tianon commented Jan 27, 2016

That Docker spot is still using v1.2 -- doh, no wonder it fails.

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.

4 participants