Comments

fubhy’s picture

A very descriptive issue summary :D

schnitzel’s picture

*g*

Isn't there an i18n Settings where you can select which textformats are able to translate? So we could use this one and if the Admin is so stupid and allows to translate PHP….

jose reyero’s picture

@Schnitzel,

Right, though that is an i18n_string setting. We'd need something like that as a global configuration option for TMGMT

schnitzel’s picture

mhh this is actually a Job of the Source Module. So why not making a Setting per Source Module where you can select with Textformats are able to translate?

berdir’s picture

Yes, source plugins will need to deal with formats themself.

However, the setting could actually live in core, though. Because the formats are the same for all sources and it would probably make sense that they are the same for all.

miro_dietiker’s picture

Version: » 7.x-1.x-dev

At least we have here a possibly security issue if we e.g. request users to translate content that possibly contains (lovely php-filtered...) PHP Code...
With auto accept, a bad external service provider (or translator) might be able to inject pretty everything... :-?

It's indeed major to deal with this cleanly to avoid later possible security issues.

schnitzel’s picture

1 day

propperdx’s picture

Component: Sources » Core

I was just wondering if anyone is working on this task. I saw that it is the next task on your roadmap.
I am looking forward to this feature.

/edit: Did I just accidentally change the component-value?

miro_dietiker’s picture

Assigned: Unassigned » berdir

Component: Where newly introduced. Selection is fine.

I guess we will discuss on how to deal with Textformats this wednesday. We're happy to hear your inputs. :-)

miro_dietiker’s picture

It seems the translation tools that are xliff based have problems with interpreting html.
SDL Trados for example supports either plain html (with masking tags) or xliff where all content is treated as content and html tags are editable by translator...

We should detect text formats and add a setting to the text format if it contains html in the origin.
This will allow us to later parse for tags and mask them.
Broken html will lead to trouble thus we should recommend/perform a validity check.

Related first
#1891840: Treat text formats differently such as HTML / markdown

Finallx xliff needs to be improved in a separate task.
#1882108: Change record: mask HTML markup in XLIFF

miro_dietiker’s picture

Doublepost..

miro_dietiker’s picture

Priority: Major » Critical
Issue summary: View changes

Promoting this task to Critical as we've seen recently systems that use PHP filter (resulting in PHP passed to translators...) and same with content containing JS.
Not that translators where a real security risks here. They simply broke PHP code (resulting in fatal errors after import) and they broke the JS logic.

But we need an answer to this before releasing stable. At least corresponding documentation.
Ans possibly output warnings (such as implement hook_requirements() to check for php module enabled...)

miro_dietiker’s picture

Issue tags: +8.x release target

I guess this time we can't ignore it...

Anushka-mp’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new2.39 KB

formatter field added for the filter_format data type, unit tests extended. setting the version to 8.x

berdir’s picture

Status: Needs review » Active
miro_dietiker’s picture

Title: Deal with text formats » Deal with text format permissions and security

Trying to find a better title to make this issue clearly different from the other similar issues...
#1891840: Treat text formats differently such as HTML / markdown

sasanikolic’s picture

In [#2472303] we found out that we get lots of errors when the translator does not have the correct permissions.

berdir’s picture

Status: Active » Needs review
StatusFileSize
new9.62 KB

This adds a setting to control which text formats are translated and which are ignored.

Together with the issue referenced above, I think we can *finally* put this issue to rest. Atleast for 8.x.

Status: Needs review » Needs work

The last submitted patch, 18: tmgmt-allowed-formats-1415234-18.patch, failed testing.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new10.64 KB
new1.03 KB

Did an unrelated fix and of course that broke a test..

  • Berdir committed 8c7eb5d on 8.x-1.x
    Issue #1415234 by Berdir: Deal with text format permissions and security
    
berdir’s picture

Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed!

And back to 7.x... Not quite sure how to fully resolve there, but we could at least backport this.

steinmb’s picture

As far as I understand, is this issue the "only" thing that blocks this module from it's first stable 7.x release?

berdir’s picture

Issue tags: -8.x release target

This is the only critical 7.x issue, so pretty much, yes. 7.x is not really a focus for us anymore and we do almost all development on 8.x, so it will either require someone to step up and port port this or sponsor a few hours of our/my time to get that done :)