Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Smarty_Internal_TemplateCompilerBase bugfix #350

Closed
tom2tom opened this issue Apr 9, 2017 · 5 comments
Closed

Smarty_Internal_TemplateCompilerBase bugfix #350

tom2tom opened this issue Apr 9, 2017 · 5 comments

Comments

@tom2tom
Copy link

tom2tom commented Apr 9, 2017

Method compileTag2() in http://github.com/smarty/libs/sysplugins/smarty_internal_templatecompilerbase.php
includes several merges into a $new_args array. The last such merge omits the array-check included in the others, which causes a crash sometimes. Lines 666 ff. should replicate the other instances in the method:

foreach ($args as $key => $mixed) {
  if (is_array($mixed)) {
    $new_args = array_merge($new_args, $mixed);
  } else {
    $new_args[ $key ] = $mixed;
  }
}
@AnrDaemon
Copy link
Contributor

I can make a blind patch, as it seems a reasonable change, but if you could write a test demonstating this issue, it would be helpful to prevent future regressions as well.

@AnrDaemon
Copy link
Contributor

After a careful look over the code, your proposed patch doesn't look valid.
I would like to see a real case that trigger your issue before attempting any changes.

@tom2tom
Copy link
Author

tom2tom commented Apr 13, 2017 via email

uwetews added a commit that referenced this issue Apr 13, 2017
@uwetews
- bugfix array_merge() parameter should be checked #350
6d85c71
@uwetews
Copy link
Contributor

uwetews commented Apr 13, 2017

The fix is now in the master branch

@uwetews uwetews closed this as completed Apr 13, 2017
@AnrDaemon
Copy link
Contributor

Shouldn't that be $new_args[] = $mixed; , since array_merge appends the numerically indexed arrays?

@uwetews uwetews mentioned this issue Apr 14, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@uwetews @AnrDaemon @tom2tom