CakeRequest::here escape optimization #527

Closed
wants to merge 36 commits into
from

Conversation

Projects
None yet
10 participants
@vantienvnn
Contributor

vantienvnn commented Feb 24, 2012

Fixed #2615

0x20h and others added some commits Jan 17, 2012

Merge pull request #416 from 0x20h/move-cache-docblock
Moved cache docblock from core.php to bootstrap.php
Fix issues with duplicate msgid values.
Msgid values could be duplicated if the same string was used
for singular and pluralized translations.  Re-index how the data is
stored so duplicate singular detection per domain is easier.

Fixes #2538
Made specifying 'extension' optional. Fixed bug where downloaded file…
… did not have extension when 'name' was specified. Fixes  #2554
Allow SqlServer to execute procs correctly.
Both SELECT and EXECUTE statements should go through
the parent method as they could fetch results.

Fixes #2558
@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Feb 10, 2012

Member

EXECUCUTE?

This comment has been minimized.

Show comment
Hide comment
@AD7six

AD7six Feb 10, 2012

Member

with a valley-girl accent? "Exec. u CUTE!"

Member

AD7six replied Feb 10, 2012

with a valley-girl accent? "Exec. u CUTE!"

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Feb 10, 2012

Member

hahahaha

Member

lorenzo replied Feb 10, 2012

hahahaha

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 10, 2012

Member

Bloody hell, I'm a fool. Sorry about that.

Member

markstory replied Feb 10, 2012

Bloody hell, I'm a fool. Sorry about that.

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 10, 2012

Member

Un-derped in [9c1fa28]

Member

markstory replied Feb 10, 2012

Un-derped in [9c1fa28]

This comment has been minimized.

Show comment
Hide comment

lol

markstory and others added some commits Feb 10, 2012

Its EXECUTE not EXECUCUTE
Also make the search insensitive as casing bugs are no fun.
Merge pull request #474 from shama/patch-numberhelper
Add missing options to NumberHelper docblocks
Fix issues with double / & leading/trailing /
Authorize classes should remove // and leading trailing /
Without this incorrect paths that fail to match nodes can be
generated.  This also allows settings[actionPath] to be
permissive in what it accepts.

Fixes #2563
Fix notice error when reading empty values.
When reading empty values a notice error would be triggered.
Slicing the first char off and comparing that solves this.

Fixes #2537
Update API docs
View switching only happens for known mime types.
Refs #2565
@0x20h

This comment has been minimized.

Show comment
Hide comment
@0x20h

0x20h Feb 12, 2012

Contributor

just to make extra sure, probably better to preg_replace('#/+#', '/', $path)

just to make extra sure, probably better to preg_replace('#/+#', '/', $path)

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 12, 2012

Member

I don't know how you'd end up with 3 slashes though.

Member

markstory replied Feb 12, 2012

I don't know how you'd end up with 3 slashes though.

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Feb 12, 2012

Member

I think this was changed by Ceeram not long ago to fix a bug in ACL, he made sure that there was a slash at the beginning of every path. What is the correct way?

Member

lorenzo replied Feb 12, 2012

I think this was changed by Ceeram not long ago to fix a bug in ACL, he made sure that there was a slash at the beginning of every path. What is the correct way?

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 12, 2012

Member

In the comments on lighthouse, I thought @ceeram mentioned that the leading slash was a problem. Ticket was http://cakephp.lighthouseapp.com/projects/42648/tickets/2563

Member

markstory replied Feb 12, 2012

In the comments on lighthouse, I thought @ceeram mentioned that the leading slash was a problem. Ticket was http://cakephp.lighthouseapp.com/projects/42648/tickets/2563

This comment has been minimized.

Show comment
Hide comment
@ceeram

ceeram Feb 12, 2012

Member

i did not change this afaik, the leading slash indeed causes issues, i discussed that a while back already in irc, it is fine as it is now

Member

ceeram replied Feb 12, 2012

i did not change this afaik, the leading slash indeed causes issues, i discussed that a while back already in irc, it is fine as it is now

This comment has been minimized.

Show comment
Hide comment
@0x20h

0x20h Feb 12, 2012

Contributor

me too @markstory. But I thought as $path is a parameter it would probably make sense to be a bit more fault-tolerant. But, of course, it's ok as it is.

Contributor

0x20h replied Feb 12, 2012

me too @markstory. But I thought as $path is a parameter it would probably make sense to be a bit more fault-tolerant. But, of course, it's ok as it is.

markstory and others added some commits Feb 12, 2012

Merge pull request #477 from krolow/ticket-2574
fixing regex of autoLinks to work with urls that have www
Fix duplicate items in HABTM associations.
Apply patch from 'Kim Biesbjerg'.  Fixes issues where nested
HABTM associations would create duplicate content.

Fixes #2564
Fixes #1598
Fix failing tests.
rawurlencode() and urlencode() handle utf8 differently.
Remove un-necessary Set::merge().
Using Set::merge() on an empty array causes issues with out of order
numeric keys. Only merge if necessary.

Fixes #2595
@ceeram

This comment has been minimized.

Show comment
Hide comment
@ceeram

ceeram Feb 22, 2012

Member

git bisect shows this commit as introduding duplicate values when posting habtm data with input('Tag') on a Post form.

request->data['Tag']['Tag'] should be array(1, 2), but now shows array(1, 2, 1, 2) when selecting first 2 tags

Member

ceeram commented on 89df484 Feb 22, 2012

git bisect shows this commit as introduding duplicate values when posting habtm data with input('Tag') on a Post form.

request->data['Tag']['Tag'] should be array(1, 2), but now shows array(1, 2, 1, 2) when selecting first 2 tags

This comment has been minimized.

Show comment
Hide comment
@ceeram

ceeram Feb 22, 2012

Member

ceeram/cakephp@2.1...2.1-requestdata

results: http://bin.cakephp.org/view/1818149545

created a testcase for this, not sure how to solve it yet, need to look into that

Member

ceeram replied Feb 22, 2012

ceeram/cakephp@2.1...2.1-requestdata

results: http://bin.cakephp.org/view/1818149545

created a testcase for this, not sure how to solve it yet, need to look into that

This comment has been minimized.

Show comment
Hide comment
@ceeram

ceeram Feb 22, 2012

Member

included a fix now as well

Member

ceeram replied Feb 22, 2012

included a fix now as well

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 23, 2012

Member

I'm an idiot, I don't know how I missed that the first time around. I merged in your changes :D

Member

markstory replied Feb 23, 2012

I'm an idiot, I don't know how I missed that the first time around. I merged in your changes :D

This comment has been minimized.

Show comment
Hide comment
@ADmad

ADmad Feb 23, 2012

Member

@markstory Your original commit was on 2.0 so perhaps the fix done by Ceeram for 2.1 should be backported to 2.0 too.

Member

ADmad replied Feb 23, 2012

@markstory Your original commit was on 2.0 so perhaps the fix done by Ceeram for 2.1 should be backported to 2.0 too.

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 23, 2012

Member

It certainly should. I cherry-picked the changes in [836f913] and [6d3c659]

Member

markstory replied Feb 23, 2012

It certainly should. I cherry-picked the changes in [836f913] and [6d3c659]

This comment has been minimized.

Show comment
Hide comment
@ceeram

ceeram Feb 23, 2012

Member

commented in wrong place already i think

@markstory The branch had 4 commits in total for the tests and fix, i messed up with making tests
you only cherry-picked two of the commits in 2.0, causing a fail

Member

ceeram replied Feb 23, 2012

commented in wrong place already i think

@markstory The branch had 4 commits in total for the tests and fix, i messed up with making tests
you only cherry-picked two of the commits in 2.0, causing a fail

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Feb 24, 2012

Member

Thanks, I guess I'll find the other commits and merge those too.

Member

markstory replied Feb 24, 2012

Thanks, I guess I'll find the other commits and merge those too.

@ceeram

This comment has been minimized.

Show comment
Hide comment
@ceeram

ceeram Feb 23, 2012

Member

@markstory the branch which fixed the issue had 4 commits, you only merged 2 of them to 2.0 causing a fail

Member

ceeram commented on 6d3c659 Feb 23, 2012

@markstory the branch which fixed the issue had 4 commits, you only merged 2 of them to 2.0 causing a fail

@lorenzo

This comment has been minimized.

Show comment
Hide comment
@lorenzo

lorenzo Feb 24, 2012

Member

Please, do not send pull request agains out master branch, use an appropriate branch.

Member

lorenzo commented Feb 24, 2012

Please, do not send pull request agains out master branch, use an appropriate branch.

@lorenzo lorenzo closed this Feb 24, 2012

@vantienvnn

This comment has been minimized.

Show comment
Hide comment
@vantienvnn

vantienvnn Feb 24, 2012

Contributor

I tried, but could not pull request on 2.0 and 2.1 branch

Contributor

vantienvnn commented Feb 24, 2012

I tried, but could not pull request on 2.0 and 2.1 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment