Bugzilla – Full Text Bug Listing
| Summary: | Limit HTML tags on Eclipse Marketplace | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Community | Reporter: | Christopher Guindon <chris.guindon> | ||||
| Component: | Marketplace | Assignee: | Darrell Armstrong <darrell.armstrong> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | darrell.armstrong, frederic.ebelshaeuser, manuel.bork, matt, tatiana.fesenko, Ted.Epstein, webdev, webmaster | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Christopher Guindon
What is with the tags : heading , <p> and <img> ? Is it possible to use this tags or something equivalent with the WYSIWYG editor ? (In reply to Frederic Ebelshaeuser from comment #1) > What is with the tags : heading , <p> and <img> ? We were planning to use the line breaks in the description but I think it should be safe to include <p> instead. Would you prefer that? Darrell, do you see a problem with replacing the line break with <p> tags? > > Is it possible to use this tags or something equivalent with the WYSIWYG > editor ? We are not planning on allowing img tags since they are vulnerable to XSS. We recommend using the "screenshot" field for all images. There are issues regarding the security of adding the img tag to Filtered HTML. https://www.drupal.org/node/347924 For clarification purposes: What about headings like h1, h2, h3, h4,... ? (In reply to Christopher Guindon from comment #2) > We are not planning on allowing img tags since they are vulnerable to XSS. > We recommend using the "screenshot" field for all images. > I understand that point with the XSS attacks. Something like HTML Purifier[1] would be a possible solution to filter the attacks out and allow img-tags. [1] http://htmlpurifier.org Created attachment 267763 [details] Screenshot of img breaking marketplace layout (In reply to Manuel Bork from comment #3) > For clarification purposes: What about headings like h1, h2, h3, h4,... ? +1 I think we should include h2, h3, h4, h5, h6 but not h1. Here's an example of how a listing will change: Before: https://marketplace.eclipse.org/content/uml-lab-modeling-ide After: https://marketplace-staging.eclipse.org/content/uml-lab-modeling-ide (In reply to Frederic Ebelshaeuser from comment #4) > (In reply to Christopher Guindon from comment #2) > > > We are not planning on allowing img tags since they are vulnerable to XSS. > > We recommend using the "screenshot" field for all images. > > > I understand that point with the XSS attacks. Something like HTML > Purifier[1] would be a possible solution to filter the attacks out and allow > img-tags. > > [1] http://htmlpurifier.org I really think we need to avoid allowing <img> tags in description for the following reasons: 1. Vulnerable to XSS 2. Consistency: Most images in the description do not include the responsive-img class and they break the layout of our site on mobile/small screens. I am attaching an screenshot of this UML Lab Modeling listing on mobile. 3. Page load time: We cannot optimize images embed in the listing description. 4. htmlpurifier(https://www.drupal.org/project/htmlpurifier) drupal module is minimally maintained. The last official release was in 2012. I really think that listings should use the screenshot field for images. Perhaps a good compromise would be to include thumbnails on the landing page of a listing instead of a separate tab? HTML tag filtering is now enabled on the Marketplace site for the content description. The current list of allowed tags are as follows: <a> <em> <strong> <u> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd> <h2> <h3> <h4> <h5> <h6> <p> <br> <strike> <pre> Please re-open this bug if you have concerns or questions Re-opening to allow for further discussion Guys, I think you should roll back this change immediately, and hold off on making further changes until you've given the larger community of Marketplace solution providers a proper voice in this discussion. Inline images and CSS formatting are really important to create visually appealing listings. Removing them significantly degrades the Eclipse Marketplace experience for users, and doesn't do justice to the solutions. While I'm sure everyone had good intentions here, this change wasn't handled well. There was no invitation into a discussion, just an announcement that listings would be restricted to a few HTML tags, to facilitate addition of a new "WYSIWYG editor." Who made the decision that it was more important to have a "WYSIWYG Editor" (which isn't even WYSIWYG) than to have rich CSS formatting and images? Which companies with product listings on the Marketplace were consulted? Who among them weighed in, and what was their feedback? This issue was opened here in Bugzilla after the announcement was made, discussed briefly, and then closed once the change was made (over a few objections). Most of us weren't even aware that the issue was opened; it wasn't communicated to me until I complained about this in an email earlier today. (Thank you Darrell for reopening this Bugzilla issue, but the issues remain...) In all of this, I haven't heard any explanation as to why such a disruptive change had to be made on an emergency basis, with little prior warning, and no proper discussion with the primary stakeholders. I see XSS vulnerabilities being discussed here, but the stated motivation for the change wasn't security; it was WYSIWYG, which is at best a convenience. I also see mention of images not being configured (in most cases) for mobile-responsive layout. Folks, these are the kinds of things you think about, and talk about in an open forum, when you're introducing a new feature or platform like Eclipse Marketplace. Or doing a new major release with breaking changes, again, with good two-way communication. These are not the kinds of arguments you introduce after the fact, in a semi-private discussion, to justify a unilateral decision you've already made. This is a _Marketplace_. That means, for some of us, there's money involved. Regardless of whether we're all counting on Eclipse Marketplace as a revenue channel, we've all invested significant time, resources and creativity into it. I take it seriously, and I really don't appreciate anyone taking shortcuts in the process of introducing a change that affects my business. Please, roll this back and let's start this process over with an open discussion. Ted Epstein, CEO RepreZen (In reply to Theodore Epstein from comment #8) > Guys, I think you should roll back this change immediately, and hold off on > making further changes until you've given the larger community of > Marketplace solution providers a proper voice in this discussion. Hi Theodore, We sent an announcement about this on March 31, 2017 and we created this bug on April 5th to open a discussion with the community. The change went live on April 20th. There was 15 days to discuss this. I believe that Eclipse bugzilla is the right location discuss these changes. > Inline images and CSS formatting are really important to create visually > appealing listings. Removing them significantly degrades the Eclipse > Marketplace experience for users, and doesn't do justice to the solutions. I would argue that we are improving the user experience for users since we are making every page more consistent. Inline images and CSS formatting is being used by less than 10% of our listings. The vast majority of the community is not affected by this. Summary counts of Marketplace db entries with img tag, inline css: field_data_body (total combined unique entries, 165 out of 1762 listings) - with img tag = 130 - with inline css on element or style tag = 80 - contain div’s = 49 Please note that invalid HTML from FULL TEXT HTML is also responsible for taking down the Eclipse Marketplace Client. Adding the following code in FULL TEXT HTML to a listing will stop MPC from loading: <IMG SRC=`javascript:alert("Eclipse says, 'XSS'")`></description> This could be added by any listing owner at anytime and if this particular listing was randomly chosen to be displayed on the landing page of MPC, MPC would crash for everyone until the cache was re-recreated and a different list of listings is chosen to be displayed on the MPC lading page. I take these issues very seriously. It's very important for us to do what we can to keep these services working. Would you agree that having MPC un-responsive has a bigger negative impact to your business than the removal of custom HTML/CSS in your listing? > > While I'm sure everyone had good intentions here, this change wasn't handled > well. There was no invitation into a discussion, just an announcement that > listings would be restricted to a few HTML tags, to facilitate addition of a > new "WYSIWYG editor." Please understand that we do have really good intentions. Our goal is to make Eclipse Marketplace secure and useful for everyone in our community. > > Who made the decision that it was more important to have a "WYSIWYG Editor" > (which isn't even WYSIWYG) than to have rich CSS formatting and images? > Which companies with product listings on the Marketplace were consulted? > Who among them weighed in, and what was their feedback? We decided to go with this message because we did not want to invite users to add Cross-site scripting (XSS) to our website while FULL HTML was enabled until April 20th. The main reason for this was security not the WYSIWYG. Here's a transcript from Eclipse Foundation infrazilla where we decided to change the email: Bug 1021 - [SECURITY] Disable the FULL TEXT html format for authenticated users (In reply to Christopher Guindon from comment #24) > Based off my concern about sending an e-mail to all maintainers saying that > our site is insecure until April 17, 2017, we've decided to update the > e-mail text to "downplay" the security aspect of this change. > > New text is available here: > https://docs.google.com/document/d/ > 1lFis1rnMUvRArLb0Mbvujt2sSyYFPCFCgPhsvXu1yXY/edit > This issue was opened here in Bugzilla after the announcement was made, > discussed briefly, and then closed once the change was made (over a few > objections). Most of us weren't even aware that the issue was opened; it > wasn't communicated to me until I complained about this in an email earlier > today. > > (Thank you Darrell for reopening this Bugzilla issue, but the issues > remain...) Did you not receive our notification about this? If we could change the way we did this, I would of created this bug before sending the email to include a link to this discussion. > > In all of this, I haven't heard any explanation as to why such a disruptive > change had to be made on an emergency basis, with little prior warning, and > no proper discussion with the primary stakeholders. I see XSS > vulnerabilities being discussed here, but the stated motivation for the > change wasn't security; it was WYSIWYG, which is at best a convenience. May I suggest you add the marketplace bugzilla inbox (marketplace-inbox@eclipse.org) to your User Watching list? This will send you an email each time a bug is open against the Marketplace Website and it's probably your best way to stay inform with Marketplace changes. > > I also see mention of images not being configured (in most cases) for > mobile-responsive layout. Folks, these are the kinds of things you think > about, and talk about in an open forum, when you're introducing a new > feature or platform like Eclipse Marketplace. Or doing a new major release > with breaking changes, again, with good two-way communication. These are not > the kinds of arguments you introduce after the fact, in a semi-private > discussion, to justify a unilateral decision you've already made. These where example of things we wanted to remove but at the end of the day, this was a security issue. All security bugs are not accessible to all users. From the Eclipse Security page: https://www.eclipse.org/security/ "Issue reports related to vulnerabilities must be marked as "committers-only", either automatically by clicking the provide link, by the reporter, or by a committer during the triage process. Note that issues marked "committers-only" are visible to all Eclipse committers. By default, a "committers-only" issue is also accessible to the reporter and individuals explicitly indicated in the "cc" list. I think where we did wrong is that the original request to disable html for authenticated users was submitted on the Eclipse Foundation private infrazilla. If this bug was submitted on bugzilla, we could of disabled the "committer-only" flag once this issue was addressed and link to that discussion from this conversation. In the future, the webdev team will make an effort to use bugzilla (https://bugs.eclipse.org) before posting a bug to the Eclipse Foundation private bugzilla instance (infrazilla). > > This is a _Marketplace_. That means, for some of us, there's money involved. > Regardless of whether we're all counting on Eclipse Marketplace as a revenue > channel, we've all invested significant time, resources and creativity into > it. I take it seriously, and I really don't appreciate anyone taking > shortcuts in the process of introducing a change that affects my business. > > Please, roll this back and let's start this process over with an open > discussion. I understand but I really think that custom html/css/images should be done on the listing website and not on Eclipse Marketplace. I would also consider Google Play store a "marketplace" where money is involved and they don't allow anyone to post custom html/css in their description. http://stackoverflow.com/questions/11071127/google-play-app-description-formatting > > Ted Epstein, CEO > RepreZen Hi Christopher, > We sent an announcement about this on March 31, 2017 and we created > this bug on April 5th to open a discussion with the community. > The change went live on April 20th. There was 15 days to discuss this. > I believe that Eclipse bugzilla is the right location discuss these > changes. Bugzilla is a fine place to discuss it, but _normally_ the issue should have been opened here first, with a full explanation of the motivations (not just "WYSIWYG Editor") and the proposal to make the change. And _normally_ the issue should have been referenced with a link in the email, so we all knew the discussion was happening. I emphasize "normally" because I understand now, having read through your explanation, that this was a sensitive security issue, and you didn't want to expose a vulnerability. Thanks for clarifying that. (FWIW, I actually looked on bugzilla before contacting you, to see if I could find this issue. I didn't find it.) > Inline images and CSS formatting is being used by less than 10% of our > listings. The vast majority of the community is not affected by this. I would think % of downloads + click-throughs might be a better measure of the change impact. > Please note that invalid HTML from FULL TEXT HTML is also responsible > for taking down the Eclipse Marketplace Client. > Adding the following code in FULL TEXT HTML to a listing will stop > MPC from loading: > > <IMG SRC=`javascript:alert("Eclipse says, 'XSS'")`></description> OK, but shouldn't it be possible to disable (or filter out) JavaScript without disabling IMG tags altogether? I assume this goes back to the HTML Purifier discussion, which is out of my area of knowledge. It just seems like this kind of attack is straightforward enough that there should be a component able to neutralize it. There are other possible measures too, like delaying updates until they've passed an automated test, running on your continuous integration platform, that verifies no breakage or security violations with the updated code. I'd appreciate input from others who know more about this. > I take these issues very seriously. It's very important for us to do > what we can to keep these services working. Would you agree that having > MPC un-responsive has a bigger negative impact to your business than the > removal of custom HTML/CSS in your listing? Absolutely. I just hope we can find a way to avoid either of those. > Please understand that we do have really good intentions. Our goal is to > make Eclipse Marketplace secure and useful for everyone in our community. I appreciate that. > We decided to go with this message because we did not want to invite users > to add Cross-site scripting (XSS) to our website while FULL HTML was > enabled until April 20th. The main reason for this was security not > the WYSIWYG. OK, I can understand that you didn't want to announce a vulnerability before you were able to plug it. It would have helped to get a follow-up communication from your team explaining that security was the primary motivation, that you understand some listings will be affected by the change, and inviting a discussion about how best to mitigate those issues. You were able to generate stats on use of IMG tags and CSS, so maybe a communication specifically to those listing providers would have been appropriate. > Did you not receive our notification about this? If we could change > the way we did this, I would of created this bug before sending the email > to include a link to this discussion. Yes, that is what I had in mind, and what I would have preferred. But again, I had not considered the impact of announcing a security problem. So the primary thing I would suggest doing differently is the follow-up communication, described above. > May I suggest you add the marketplace bugzilla inbox > (marketplace-inbox@eclipse.org) to your User Watching list? > This will send you an email each time a bug is open against the > Marketplace Website and it's probably your best way to stay inform > with Marketplace changes. Done, and thanks. > I think where we did wrong is that the original request to disable html > for authenticated users was submitted on the Eclipse Foundation private > infrazilla. If this bug was submitted on bugzilla, we could of disabled > the "committer-only" flag once this issue was addressed and link to that > discussion from this conversation. > > In the future, the webdev team will make an effort to use bugzilla > (https://bugs.eclipse.org) before posting a bug to the Eclipse > Foundation private bugzilla instance (infrazilla). OK, sounds reasonable. > ...I really think that custom html/css/images should be done on the > listing website and not on Eclipse Marketplace. I would also consider > Google Play store a "marketplace" where money is involved and they > don't allow anyone to post custom html/css in their description. > http://stackoverflow.com/questions/11071127/google-play-app-description-formatting I think part of what makes it work for other marketplaces is the way screenshots are integrated into the UX, not segregated to another tab. I'll reply back later with some thoughts on this. Ted Christopher & Team, I looked at the Google Play store, and also the iOS App Store. Both are highly visual presentations, with a very similar layout. Images are at the top, before the text content, in a combination slider/lightbox control, with an embedded video player. https://play.google.com/store/apps/details?id=com.fungamesforfree.colorfy&hl=en Static images, animated GIFs and videos are engaging, and can sometimes impart a lot of information about the listed product, more quickly and intuitively than text. This kind of layout also allows for providers to differentiate their products visually, while still enforcing consistency in the the layout, typography, etc. But would you consider making a change like this, giving images (maybe as an option) prominent display along with the text description, rather than requiring users to click into a separate tab? If not, would you consider options to allow images in the description content, safely? - Ted (In reply to Theodore Epstein from comment #11) > Christopher & Team, > > I looked at the Google Play store, and also the iOS App Store. Both are > highly visual presentations, with a very similar layout. Images are at the > top, before the text content, in a combination slider/lightbox control, with > an embedded video player. > > https://play.google.com/store/apps/details?id=com.fungamesforfree. > colorfy&hl=en > > Static images, animated GIFs and videos are engaging, and can sometimes > impart a lot of information about the listed product, more quickly and > intuitively than text. > > This kind of layout also allows for providers to differentiate their > products visually, while still enforcing consistency in the the layout, > typography, etc. > > But would you consider making a change like this, giving images (maybe as an > option) prominent display along with the text description, rather than > requiring users to click into a separate tab? > > If not, would you consider options to allow images in the description > content, safely? > > - Ted Hi Theodore, I totally agree with you on this. Mp listings screenshots are currently hidden and I think it would be a great improvements if we could included them on the landing page of a listing. I could also see a new field that will allow MP listings to include the url of a youtube video. The video would be embedded automatically on the page. How about I ask one of our graphic designers to design a mockup of a MP listing with the screenshots on the landing page + an optional embedded youtube video? Hi Christopher,
> How about I ask one of our graphic designers to design a mockup of a MP
> listing with the screenshots on the landing page + an optional embedded
> youtube video?
Sounds like it would be a welcome improvement, and a big step towards modernization of Eclipse Marketplace.
In the spirit of community-driven software, I'd suggest opening a new issue for this. And if it progresses, maybe send an update to all active Marketplace listing providers to let them know this discussion is happening, so they have an opportunity to participate. Just a thought. ;-)
Thanks,
Ted
(In reply to Theodore Epstein from comment #13) > Hi Christopher, > > > How about I ask one of our graphic designers to design a mockup of a MP > > listing with the screenshots on the landing page + an optional embedded > > youtube video? > > Sounds like it would be a welcome improvement, and a big step towards > modernization of Eclipse Marketplace. > > In the spirit of community-driven software, I'd suggest opening a new issue > for this. I created Bug 516134 - Improve marketplace listing layout https://bugs.eclipse.org/bugs/show_bug.cgi?id=516134 for this. And if it progresses, maybe send an update to all active > Marketplace listing providers to let them know this discussion is happening, > so they have an opportunity to participate. Just a thought. ;-) > > Thanks, > > Ted I agree we should send a follow up email about this bug to MP listing owner. I don't see the need for contacting MP listing owners about new bugs but perhaps I could include instructions on how to follow marketplace website issues on bugzilla in my follow up email? > I agree we should send a follow up email about this bug to > MP listing owner. > > I don't see the need for contacting MP listing owners about > new bugs but perhaps I could include instructions on how to > follow marketplace website issues on bugzilla in my follow > up email? That's a good idea. It's actually hard to find where Marketplace-related issues are being discussed on Bugzilla. But I'll open a separate issue for that. Closing this bug! The change went live in 2017. |