Images are not shown properly in the PMI (project proposals)

Summary

Images of this draft of project proposal are not shown properly in the content of the proposal, however they are visible in the "edit" mode.

Steps to reproduce

Open the proposal draft. Select the Edit tab and see the images (inserted in the description section).

What is the current bug behavior?

Images are not shown after saving uploading them in the edit mode.

What is the expected correct behavior?

Images added in edit mode should be visible in the drafted proposal, or at least the user should be warned about the issue that is preventing the PMI to show the images.

moved from eclipsefdn/helpdesk#835 (moved)

added team:webdev label

assigned to @epoirier

I created the following MR to add the <img> tag to the filtered html formats on the PMI https://gitlab.eclipse.org/eclipsefdn/it/webdev/drupal/eclipse_pmi/-/merge_requests/2

I reverted my changes because this allows people to use the <img> to inject XSS to the site. We need to figure out a better way to allow the <img> tag to be used within body tags of the site.

Adding more context: I was sick for the last few days and I was not around when this change was made.

I asked Eric to revert this change since there are some security risks in allowing the img tag to all authenticated users on our sites.

The default "Filtered HTML" text filter that ships with Drupal do not allow the <img> tag because image tags can contain attributes that allow for XSS attacks.

If this is a feature that we need, there might be a way as long as we prevent untrusted users from using images as a vector for XSS attacks.

A possible solution would be to install the WYSIWYG Filter module: https://www.drupal.org/project/wysiwyg_filter

https://www.drupal.org/files/issues/wysiwyg_filter_settings.png

However, this project is not covered by the security advisory policy. We have to use at our own risk!

@mdelgado624 On a scale from 1 to 10, can you tell us how important is it for you that we allow any user to insert images on projects.eclipse.org?

@cguindon I guess project proposals could work without images, although I do find images to be important (7) when explaining for example a platform software architecture.

In any case, if images are not supported for drafting project proposals this should be made clear to the users avoiding them to upload resources that will not be available for the community to access.

In any case, if images are not supported for drafting project proposals this should be made clear to the users avoiding them to upload resources that will not be available for the community to access.

I took a look and you are right, the filter HTML text format was including an icon that allowed users to insert images even if that feature is not supported by that text filter.

This was caused by a conflict in our WYSIWYG configs. I deleted all duplicates. Now both the Filter HTML and Filtered HTML (Covert line breaks into HTML) filters are displaying the same options:

There is also a link under each section where the text filter is used that links to a page where we explain the limitation of each text filter: https://projects.eclipse.org/filter/tips

added Feature Request label

In any case, if images are not supported for drafting project proposals this should be made clear to the users avoiding them to upload resources that will not be available for the community to access.

An alternative solution would be to add a new screenshot field on the project proposal form where we allow users to upload image/screenshots. We can then display them under a new screenshot section on the page.

Would that be a good compromise?

@epoirier,

@mdelgado624 and I spoke and before we make any decisions, we would like to better understand how many projects are impacted by this.

Would it be possible for you to provide a list of all the projects using an IMG tag in the description and scope field?

Ok no problem, I'll have a look at that this week.

Just to point you in the right direction, I would query the database in order for you to find an img tag in one of those fields.

Note that we only want to query Project nodes.

That was my inital plan as well.

There are a total of 59 projects using an <img> tag in their description.

You can see the report here:

projects_using_img.csv

Can you share the query? I want to make sure you are checking the correct revisions.

This is the query that I'm using:

SELECT d.entity_id as nid, n.title, n.type, d.body_value as description from dev_drupal7_projects.field_data_body as d LEFT JOIN dev_drupal7_projects.node as n on d.entity_id = n.nid where body_value LIKE '%<img%' AND n.type = "project";

This query is searching all the revisions but we only need to check the latest one: https://www.drupal.org/node/895736

Here is the new query with the same result of 59 projects.

SELECT d.entity_id as nid, n.title, n.type, d.body_value as description from dev_drupal7_projects.field_revision_body as r LEFT JOIN dev_drupal7_projects.field_data_body as d on d.revision_id = r.revision_id LEFT JOIN dev_drupal7_projects.node as n on d.entity_id = n.nid where r.body_value LIKE '%<img%' AND n.type = "project";

I was hoping that number was going to go down

Actually, I think there is still something wrong with your query.

I took a look at the first entry in your report which is node 473: https://projects.eclipse.org/projects/modeling.emft.ecoretools/edit

The content in your report does not match what I see on production (wich is expected since your first query did not look up revisions). More importantly, I don't see any images in the description or scope of https://projects.eclipse.org/projects/modeling.emft.ecoretools/edit

Ok my local copy of the database is recent but I'll pull a new one and see if I get different results.

I see what's wrong with your query. You are still not joining on the latest revision.

SELECT d.entity_id  AS nid,
       n.title,
       n.type,
       d.body_value AS description
FROM   dev_drupal7_projects.field_revision_body AS r
       LEFT JOIN dev_drupal7_projects.field_data_body AS d
              ON d.revision_id = r.revision_id
       LEFT JOIN dev_drupal7_projects.node AS n
              ON d.entity_id = n.nid
WHERE  r.body_value LIKE '%<img%'
       AND n.type = "project"; 

This is your last query. I formatted it to make it easier to read for me.

This query is looking up all the field_revision_body values.

Ok here's my updated query:

SELECT n.nid, b.body_value as description
FROM dev_drupal7_projects.node as n
LEFT JOIN dev_drupal7_projects.field_data_body as b on b.entity_id = n.nid and n.vid = b.revision_id
where b.body_value LIKE '%<img%'
AND n.type = "project";

Now i'm getting 47 results

projects_images.csv

I had a look at ~10 projects from this last report and they all have images in their description on production.

Interesting enough, some project pages currently do display images: https://projects.eclipse.org/node/861

Based on what I can see, someone with admin access added the img tag: https://projects.eclipse.org/admin/structure/features/eclipse_pmi/diff

- Chris

Mystery solved. The eclipse_pmi module was not properly updated and a recent update reverted the changes to bring back img.

I've asked @epoirier to look into the contrib module that I was suggesting to see if this is even a possible option for us: #19 (comment 503565)

@cguindon I tested the WYSIWYG Filter module locally and it does what we expect it to do.

You select attributes you'd like to whitelist and the rest of the attributes are stripped out on save. So the unwanted attributes are not saved in the database. Not sure this is a good thing or not since we try to always save whatever the user add to the form.

Interesting. There is no way to configure this module to filter on display instead than on save?

Is it possible to only apply this filtering to img? The only attributes that I would allow are src, title and alt for now.

The plugin allows us to create a list of tags and their attributes that we want to whitelist. So in other words we need to add all the tags that are listed in the "Allowed HTML tags" section.

Here is a screenshot of the settings I have locally. For this example I whitelisted the src, width and height attributes for the img tag.

I like that this module prevents us to add HTML tags that could have security issues (see in the Additional Notes of the screenshot).

Perfect! I think the next step would be to install this on staging so that @mdelgado624 and I can test the behaviour to allow us to make a decision on the best path forward.

Can you push this on staging before the end of next week, please?

Ok no problem, will do. I'll add that to my list.

@cguindon I added the module to staging and you can play around with the settings here: https://projects-staging.eclipse.org/admin/config/content/formats/full_html

@mdelgado624 I added an image on the following page: https://projects-staging.eclipse.org/projects/modeling.emft.emf-store

I tested with the attributes that @cguindon suggested we use. You can also try and add more attributes to see that they are not accepted.

Let me know if you have any questions.

@epoirier I cannot access https://projects-staging.eclipse.org/projects/modeling.emft.emf-store, anything I should do?

to access a page on any staging site, you have to enter the following credentials:

username: testuser

password: plaintext

@cguindon I updated the list of accepted tags and their attributes on staging.

I updated a few projects for testing and all the tags that were used in the description of those projects remainded in the field.

added to epic &14 (closed)

added statetodo label

added prioritymedium severitylow labels

marked this issue as related to #162 (closed)

removed from epic &14 (closed)