Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I had this error come up repeatedly in /sites/all/modules/ubercart/uc_store/uc_store.module on line 2042. after I updated to Ubercart 5.x-1.10.
So I went into the uc_store.module and changed the following lines:
Line 2040:
$match = array('/^
/', '/
$/', '/
(\s*|[\s*
\s*]+)
/', '/
/', '`
, N/A`');
to
$match = array('/^
/', '/
$/', '/
(\s*|[\s*
\s*]+)
/', '/
/', '`
, N/A`');
Line 2041:
$replace = array('', '', '
', '
', '', '');
to
$replace = array('', '', '
', '
', '', '');
Don't quite know if I did the right thing but the error seems to have disappeared.
Comment | File | Size | Author |
---|---|---|---|
#6 | 880908.patch | 919 bytes | TR |
Comments
Comment #1
armetrix CreditAttribution: armetrix commentedComment #2
kdebaas CreditAttribution: kdebaas commentedCould you post a patch, or wrap your changes in
code
tags? It's impossible to see what it is that you have changed exactly. But it looks like the changes of #421366: Invalid XHTML tags needed to escape some of the forward slashes.Comment #3
ledom CreditAttribution: ledom commentedSame issue here.
Solved by using old version (1.9) of lines 2040 and 2041
Old version (1.9)
New (1.10)
Comment #4
TR CreditAttribution: TR commented(Please don't use "needs review" unless there's an actual patch to review!)
Here's the patch. Please test.
Be aware that when Drupal 5 becomes unsupported, Ubercart 1.x will also be unsupported. That's probably going to happen in the next month. So I advise you to begin upgrading as soon as possible.
Comment #5
Druid CreditAttribution: Druid commentedRef: #3
If strings such as '/^<br />/' are being passed to preg functions as search patterns, aren't you going to have problems with the middle "/" ending the pattern? That's what the original error message is stating. Wouldn't it have to be '/^<br \/>/' or '#^<br />#'? You are using / as a pattern delimiter, so any / within the pattern would need to be escaped.
P.S. Is it definitely <br /> in all cases, or could there be <br/> in some cases? Either is valid XHTML — perhaps the search pattern should be <br */> (0 or more spaces before /) or even <br */?> (optional self-closing)?
Comment #6
TR CreditAttribution: TR commentedForgot to attach the patch ... patch changes the Ubercart 1.x version of uc_store.module to use the same pattern matching as the Ubercart 2.x version.
Comment #7
kdebaas CreditAttribution: kdebaas commentedApplied patch, and watchdog messages went away. Would you like me to test anything else?
Comment #8
whoey CreditAttribution: whoey commentedsame here, patch seems to have sorted the issue Cheers!
Comment #9
TR CreditAttribution: TR commentedOK, patch committed.
Comment #10
j0rd CreditAttribution: j0rd commentedSame issue. Applied patch. Problem Solved.
Many businesses won't have the man hours or funds upgrade right away, so at least implementing simple bug fixes back into the tree would be nice.
I personally just took on a Ubercart 1.x / Drupal 5.x project...but I think I'll be holding out on an upgrade to D6/UC2 and instead just wait a couple extra months and jump right to D7 / UC3. That makes more economic sense for most companies in the long run I think.
Comment #12
frost CreditAttribution: frost commented@j0rd:
i think that if you're doing a "normal" upgrade, you'll still have to upgrade to 6 along the way to 7. some people might just rebuild the site starting in 7, but i suppose it's a balance of the amount of work you'll need to do in either case. upgrades aren't always problem free ;-). i'm just now upgrading a 5.x site with ubercart to 6.x even tho i know there'll be a 7.x upgrade sometime in the next year (depending on when all the dependent modules are available for 7)
Comment #13
shaven CreditAttribution: shaven commentedDruid wins!!! He is absolutely right. Here is what it needs to look like:
$match = array('/^<br \/>/', '/<br \/>$/', '/<br \/>(\s*|[\s*<br \/>\s*]+)<br \/>/', '/<br \/><br \/>/', '`<br \/>, N/A`');
$replace = array('', '', '<br />', '<br />', '', '');
Please update the patch.
Comment #14
kingandy CreditAttribution: kingandy commented@shaven: The '/' mid-string token was the cause of the initial issue ("Unknown modifier '>'" - the '>' being the '>' character after the '/'). The patch surrounds the '/' with brackets ('( /)?'), apparently in order to make it optional; this appears to have the happy side effect of fixing the preg_replace error, as the parser does not consider a '/' inside a bracketed clause to denote the end of the regexp. The patch works correctly.
Comment #15
Druid CreditAttribution: Druid commentedRe: #13 and #14
If it works, it works. I would point out that you don't have to use / as the delimiter, and escape it within the pattern. The
$match
could have been$match = array('#^<br />#', '#<br />$#', '#<br />(\s*|[\s*<br />\s*]+)<br />#', '#<br /><br />#', '`<br />, N/A`');
(not certain about that last element -- are backticks just being used as the PREG delimiter?). As
<br/>
is also legitimate, you may want to allow 0 or more spaces between the br and the /. There's also the issue of plain<br>
to consider.I think the Perl Regexp implementers really missed the boat on this one. They could have scanned left-to-right for the delimiter (pick up /), then scanned right-to-left from the right end of the string to pick up the flags and find the closing delimiter. This would leave everything in between as the pattern, without having to worry about escaping the delimiter which happens to be in the pattern. They might have even dropped the explicit delimiters (as ereg did) and add them as a separate argument, or reserved a certain character as an end-of-pattern separator to mark that flags are coming up at the end of the string. In that case, the separator would have to be escaped if it occurred in the pattern, so maybe nothing's gained in the end.
etc. Any way, it's still incompatible with ereg, so it's debatable as to whether anything is gained with this syntax.
Comment #16
resting CreditAttribution: resting commentedi downloaded 5.x-1.10 released on 2010-Aug-11. Had the same error.
Applied this patch and the error is gone.
Can it be committed to the main package?
Comment #17
Drupal Centric CreditAttribution: Drupal Centric commentedPatch also worked for me using 5.x-1.10, thanks.