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.
The text "{Some math: 1 + 1 = 2}" was not replaced by the actual rendering of the module you created at steps 3 to 5.
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.
Status | New | ⇒ | Confirmed |
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.
$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()
.
str_replace has a count parameter, too.
count: If passed, this will be set to the number of replacements performed.
That is a count of the number of replacements. That's different from the $limit
parameter of preg_replace()
.
Yes, you're right.
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.
@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
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.
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?
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?
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".
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...
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.
Labels |
Added:
J3 Issue
|
@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)
Status | Confirmed | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-09-01 15:49:20 |
Closed_By | ⇒ | franz-wohlkoenig |
Closed_Date | 2018-09-01 15:49:20 | ⇒ | 2018-09-01 15:49:21 |
Closed_By | franz-wohlkoenig | ⇒ | joomla-cms-bot |
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/21329
@franz-wohlkoenig
Could you please reopen here. Thank you.
Status | Closed | ⇒ | New |
Closed_Date | 2018-09-01 15:49:21 | ⇒ | |
Closed_By | joomla-cms-bot | ⇒ |
Reopened as requested
Status | New | ⇒ | Discussion |
Closed_By | franz-wohlkoenig | ⇒ | joomla-cms-bot |
Status | Discussion | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-09-07 08:29:35 |
Closed_By | ⇒ | franz-wohlkoenig |
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/21329
Issue confirmed.
http://wohlkoenig-3.joomla.com/index.php/en/blog/10-21329