J3 Issue ?
avatar Rosie-Eagle
Rosie-Eagle
31 Jul 2018

Steps to reproduce the issue

  1. Log in at the backend of joomla
  2. Go to menu "Extensions > Modules".
  3. Create a "Custom" module.
  4. Give field 'title' the following value (without the quotes): "Some math: 1 + 1 = 2"
  5. Save the module.
  6. Next create a new article (or edit an existing one).
    Make sure you can later view the article at the fronted of joomla, where visitors normally view the article.
  7. In the WYSIWYG-editor of the article mentioned at step 6 above, where you create your article content, click the '+module' button.
  8. Select your custom module which you created at steps 3 to 5 above.
  9. You should now see a text (without quotes) "{Some math: 1 + 1 = 2}" beeing inserted in your article content.
  10. Save the article.
  11. View the article at the frontend of joomla.
    Notice "{Some math: 1 + 1 = 2}" not beeing replaced by the actual rendering of the
    module you created at steps 3 to 5. The text "{Some math: 1 + 1 = 2}" is still there
    as if it were untouched.

Expected result

The text "{Some math: 1 + 1 = 2}" should have been replaced by the actual rendering of the module you created at steps 3 to 5.

Actual result

The text "{Some math: 1 + 1 = 2}" was not replaced by the actual rendering of the module you created at steps 3 to 5.

System information (as much as possible)

The 'bug' is located in file plugins/content/loadmodule/loadmodule.php at line 116, which states:

$article->text = preg_replace(addcslashes("|$matchmod[0]|", '()'), addcslashes($output, '\\$'), $article->text, 1);

The problem here is that Perl Regular Expressions special characters are not properly dealt with (by the use of addcslashes in the first parameter of preg_replace).

Changing the line mentioned above into the one below would solve the problem (at least it works for me in my situation):

$article->text = preg_replace('|'.preg_quote($matchmod[0]).'|', addcslashes($output, '\\$'), $article->text, 1);

'preg_quote' now correctly deals with Perl Regular Expressions special characters.

Additional comments

avatar Rosie-Eagle Rosie-Eagle - open - 31 Jul 2018
avatar joomla-cms-bot joomla-cms-bot - labeled - 31 Jul 2018
avatar franz-wohlkoenig franz-wohlkoenig - change - 1 Aug 2018
Status New Confirmed
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 1 Aug 2018

Issue confirmed.

  • Plugin "Content - Load Modules" is enabled,
  • "Prepare Content" in Custom Module is set on "Yes".

screen shot 2018-08-01 at 11 53 18
http://wohlkoenig-3.joomla.com/index.php/en/blog/10-21329

avatar ReLater
ReLater - comment - 2 Aug 2018

Related: #11065

I can confirm the issue and that the fix works. I also would use preg_quote.

One could also extend the charlist of addcslashes.

What I've never understood why we don't use something like

$article->text = str_replace($matchmod[0], $output, $article->text, 1);

$matchmod[0] contains the exact pattern (string) that shall be replaced. Why a preg_replace.

avatar mbabker
mbabker - comment - 2 Aug 2018

$matchmod[0] contains the exact pattern (string) that shall be replaced. Why a preg_replace.

The comment directly above the line, "We should replace only first occurrence...". You can't do that with str_replace().

avatar mbabker mbabker - change - 2 Aug 2018
The description was changed
avatar mbabker mbabker - edited - 2 Aug 2018
avatar ReLater
ReLater - comment - 2 Aug 2018

str_replace has a count parameter, too.

count: If passed, this will be set to the number of replacements performed.

avatar mbabker
mbabker - comment - 2 Aug 2018

That is a count of the number of replacements. That's different from the $limit parameter of preg_replace().

avatar ReLater
ReLater - comment - 2 Aug 2018

Yes, you're right.

avatar Rosie-Eagle
Rosie-Eagle - comment - 2 Aug 2018

@ReLater

You wrote "One could also extend the charlist of addcslashes."

Although this is certainly true (if properly implemented), there are 2 things you should be aware of if you choose to fix the issue that way (they are the reasons why I would not recommend this approach):

a. Consider what happens if you have a "|" character (without quotes) in your title, while using the code below (don't mention the characters in the charlist-code below, some of which are not beeing (properly) escaped, focus on the issue at hand / the point I'm trying to make as I didn't check my code carefully, sorry for that):
addcslashes("|$matchmod[0]|", ".\+?[^]$(){}=!<>|:-")
I think you get my point: it should instead be done this way (because else you would also escape the beginning end ending "|" of the regular expression):
addcslashes("|".$matchmod[0]."|", ".\+
?[^]$(){}=!<>|:-")
So this is actually not an issue if properly implemented.
b. The other problem however could be an issue. What if (possibly this is only a theoretical problem, not a problem which occurs in reality) Perl Regular Expressions get extended in the future, so that there will be more special characters to be escaped?
In case of using "addcslashes" you would have to fix your code (again).
In case of using "preg_quote" one may assume that this function will also be updated so it will also escape those added special characters.

Thanks for carefully looking at the issue. As I can see you noticed that this issue is related to one which was posted in 2016. I personally think it's better to solve this issue once and for all and reconsider and test the solution properly instead of altering the problem but in another form reappearing in next releases of joomla.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21329.

avatar ReLater
ReLater - comment - 2 Aug 2018

@Rosie-Eagle
Yes, that's why I have written

I also would use preg_quote.

in comment #21329 (comment)

And why I didn't start a pr with following endless discussions. Some guys know why they want to use unreliable addcslashes(). Me not ;-) (because I don't see any significant performance issues in this case if we would use preg_quote() for the $pattern part )

You just have to provide a pr. If you don't know how:
https://docs.joomla.org/Using_the_Github_UI_to_Make_Pull_Requests

avatar Rosie-Eagle
Rosie-Eagle - comment - 2 Aug 2018

@ReLater

A pr won't be neccessary for me, I'm satisfied with your answer.
The choice of words you made earlier e.g. "I would also ..." triggered my brain to interpret this as "I'm not / might not be the one who actually alters the source-code and commits the change in the main-branch, but if it happens to be me, I would also ...".

You see, I was already convinced that if it were you, that the issue would be solved correctly.
But my interpretation above told me to go after this, just to make sure that if it were not you who's eventually solving the problem in the source-code, the guy who's responsible for it would also know why in your and my opinion we choose to solve it with 'preg_quote' and not with 'addcslashes' or something else. If he chose an alternative then there's a chance I encounter this or some derived problem in the future, which is not what I (and probably you too) want. So for me the issue is marked as "solved" now and I will wait to see the fix in the coming update-release of joomla. Thank you for properly investigating this.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21329.

avatar Rosie-Eagle
Rosie-Eagle - comment - 2 Aug 2018

I'm sorry. The day before yesterday I created a github-account, so I could post the joomla problem here.
So basically this is my first problem I've ever posted and I'm willing to contribute / do what is neccessary to get the issue "solved", but I don't seem to fully understand what I have to do to get there. I get the idea of a community where no one is responsible for the code, because we all (as a community) are. As far as I've read on the pr-page which @ReLater provided, there's a community and a core-team. I'm a developer myself, but I'm not familiar with the concept and workflow of "community and core-team". I assumed that I was part of the community and that by posting the problem above I could get into contact with the core-team, so they (you) would know the neccessary steps to be taken and follow this issue through. However, according to the last mail I got from @mbabker it seems to me that I was wrong in my assumption(s). I didn't mean to hold anyone responsible for the source-code, I just wanted to get this issue properly dealt with by whoever would eventually makes the change(s) in the source-code (didn't want to blaim in case of handling incorrectly, just wanted to let the suggestions and information I had reach the person who eventually makes the change(s)).

Now according to the mail of @mbabker a pr is neccessary, because without one nothing gets changed.
On the pr-page I read the following: "A hastily proposed pull request can hurt its chances of acceptance".
This makes my question if it is a good idea to let me generate such a request?
Not that I'm not willing to do that (I would do it if I knew howto do it (correctly)), but as said, I'm a newbie here so chances are that I'm doing this wrong.

My question to @mbabker and @ReLater now is: can you guys perhaps help me with this?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21329.
avatar Rosie-Eagle
Rosie-Eagle - comment - 2 Aug 2018

I'm sorry. The day before yesterday I created a github-account, so I could post the joomla problem here.
So basically this is my first problem I've ever posted and I'm willing to contribute / do what is neccessary to get the issue "solved", but I don't seem to fully understand what I have to do to get there. I get the idea of a community where no one is responsible for the code, because we all (as a community) are. As far as I've read on the pr-page which @ReLater provided, there's a community and a core-team. I'm a developer myself, but I'm not familiar with the concept and workflow of "community and core-team". I assumed that I was part of the community and that by posting the problem above I could get into contact with the core-team, so they (you) would know the neccessary steps to be taken and follow this issue through. However, according to the last mail I got from @mbabker it seems to me that I was wrong in my assumption(s). I didn't mean to hold anyone responsible for the source-code, I just wanted to get this issue properly dealt with by whoever would eventually makes the change(s) in the source-code (didn't want to blaim in case of handling incorrectly, just wanted to let the suggestions and information I had reach the person who eventually makes the change(s)).

Now according to the mail of @mbabker a pr is neccessary, because without one nothing gets changed.
On the pr-page I read the following: "A hastily proposed pull request can hurt its chances of acceptance".
This makes my question if it is a good idea to let me generate such a request?
Not that I'm not willing to do that (I would do it if I knew howto do it (correctly)), but as said, I'm a newbie here so chances are that I'm doing this wrong.

My question to @mbabker and @ReLater now is: can you guys perhaps help me with this?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21329.
avatar mbabker
mbabker - comment - 2 Aug 2018

In essence the "core team" in Joomla are the people with merge rights on this repo, putting out releases, or other higher level maintenance tasks. Sure, some of the "core team" also does regular bug fixing and new feature work, but they don't do that because they are part of the "core team".

Anyone is welcome to submit a bug report, a feature proposal, a pull request with a code change, etc. If you can code, pull requests fixing bugs either you've experienced or that others have reported that nobody has worked on a fix for yet are most desired. If you can't, as long as it's on this issue tracker then at least the bug is reported and acknowledged and someone who does code can come along and work on a fix.

I don't know where or why we keep referring to a "core team", but for all intents and purposes there really isn't one. There are some trusted people who are "gatekeepers" to our code repositories who review and merge things (because we don't want to just accept everything thrown at us, and we want to make sure what is accepted actually works, doesn't have issues, doesn't create security holes, etc.) and put out releases, but IMO that's really the extent of that "team".

avatar Rosie-Eagle
Rosie-Eagle - comment - 2 Aug 2018

Ok, I generated a Pull Request and referred to this thread / link for more information.
So basically the only thing I need to know now is: will I get notified if the change is accepted or rejected in the main (staging) branch or not? If it is rejected and I'm aware of it, somebody else can create another Pull Request. Thanks for the help...


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21329.

avatar mbabker
mbabker - comment - 2 Aug 2018

You need to open the pull request against this repo, not your personal fork.

And yes, you'll get notifications on the pull request the same way you're getting notifications on this issue. GitHub takes care of all that email stuff and opts you in to notifications by default on an item you open.

avatar ReLater
ReLater - comment - 2 Aug 2018

@Rosie-Eagle

  • Go to https://github.com/joomla/joomla-cms
  • Tabulator "Code"
  • Check that the Branch selector shows "staging" (in this case that's the right branch).
  • Then you're in the relevant repo and you can follow the edit steps of the tutorial/doc.
  • Don't forget step "Send Pull Request".
avatar brianteeman brianteeman - change - 2 Aug 2018
Labels Added: J3 Issue
avatar brianteeman brianteeman - labeled - 2 Aug 2018
avatar ReLater
ReLater - comment - 22 Aug 2018

@Rosie-Eagle
Will you try again to commit against staging branch or shall someone else take over your bug fix from Rosie-Eagle#1 ?
See my comment #21329 (comment)

avatar franz-wohlkoenig franz-wohlkoenig - change - 1 Sep 2018
Status Confirmed Closed
Closed_Date 0000-00-00 00:00:00 2018-09-01 15:49:20
Closed_By franz-wohlkoenig
avatar joomla-cms-bot joomla-cms-bot - change - 1 Sep 2018
Closed_Date 2018-09-01 15:49:20 2018-09-01 15:49:21
Closed_By franz-wohlkoenig joomla-cms-bot
avatar joomla-cms-bot joomla-cms-bot - close - 1 Sep 2018
avatar joomla-cms-bot
joomla-cms-bot - comment - 1 Sep 2018
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 1 Sep 2018

closed as having Pull Request #21948

avatar ReLater
ReLater - comment - 3 Sep 2018

@franz-wohlkoenig
Could you please reopen here. Thank you.

avatar brianteeman brianteeman - change - 3 Sep 2018
Status Closed New
Closed_Date 2018-09-01 15:49:21
Closed_By joomla-cms-bot
avatar brianteeman brianteeman - reopen - 3 Sep 2018
avatar brianteeman
brianteeman - comment - 3 Sep 2018

Reopened as requested

avatar franz-wohlkoenig franz-wohlkoenig - change - 4 Sep 2018
Status New Discussion
avatar SharkyKZ
SharkyKZ - comment - 7 Sep 2018

Test PR #22034 please.

avatar joomla-cms-bot joomla-cms-bot - change - 7 Sep 2018
Closed_By franz-wohlkoenig joomla-cms-bot
avatar joomla-cms-bot joomla-cms-bot - close - 7 Sep 2018
avatar franz-wohlkoenig franz-wohlkoenig - change - 7 Sep 2018
Status Discussion Closed
Closed_Date 0000-00-00 00:00:00 2018-09-07 08:29:35
Closed_By franz-wohlkoenig
avatar joomla-cms-bot
joomla-cms-bot - comment - 7 Sep 2018
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 7 Sep 2018

closed as having Pull Request #22034

Add a Comment

Login with GitHub to post a comment