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

Only allow HTTP(S) URLs in example app #2003

Merged
merged 1 commit into from
Jun 17, 2021

Conversation

pablobm
Copy link
Collaborator

@pablobm pablobm commented Jun 17, 2021

We may want to clean the example app's DB after merging this.

@pablobm pablobm requested a review from nickcharlton June 17, 2021 16:57
Copy link
Member

@nickcharlton nickcharlton left a comment

Choose a reason for hiding this comment

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

One thought, but otherwise looks good!

:product,
image_url: "\n https://example.com/foo/bar \n",
)
product = Product.create(attrs)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
product = Product.create(attrs)
it "is trimmed on save" do
product = create(
:product,
image_url: "\n https://example.com/foo/bar \n",
)

I don't seem to be able to get the suggestion to work out, but I hope you get the idea 😅

What do you think about collapsing all of these together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I don't know why I went with that. It's something about FactoryBot not doing anything clever behind my back, but it's a bit silly. OK, I'll fix it 👍

@pablobm pablobm merged commit e1baea3 into thoughtbot:main Jun 17, 2021
@pablobm pablobm deleted the fix-example-xss branch June 17, 2021 21:18
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