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.

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

When page param == 0, it gets ignored #2355

Closed
wdebusschere opened this Issue Feb 20, 2015 · 35 comments

Comments

Projects
None yet
4 participants
@wdebusschere

wdebusschere commented Feb 20, 2015

page param 0 gets ignored

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Could you be a little more specific ?

The name of your param is 0 ?

Member

nitriques commented Feb 20, 2015

Could you be a little more specific ?

The name of your param is 0 ?

@wdebusschere

This comment has been minimized.

Show comment
Hide comment
@wdebusschere

wdebusschere Feb 20, 2015

Nitriques, value in the url for the param is 0.
Example www.mysite.com/products/0/45
I need to pass 2 parameters, sometimes the first one is empty..

wdebusschere commented Feb 20, 2015

Nitriques, value in the url for the param is 0.
Example www.mysite.com/products/0/45
I need to pass 2 parameters, sometimes the first one is empty..

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques
Member

nitriques commented Feb 20, 2015

What do you get it you add `var_dump($this->param)die; at this line https://github.com/symphonycms/symphony-2/blob/integration/symphony/lib/toolkit/class.frontendpage.php#L422 ?

@wdebusschere

This comment has been minimized.

Show comment
Hide comment
@wdebusschere

wdebusschere Feb 20, 2015

I get NULL and Cannot modify header information - headers already sent by if i dont add "die",
with die i get blank screen

wdebusschere commented Feb 20, 2015

I get NULL and Cannot modify header information - headers already sent by if i dont add "die",
with die i get blank screen

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Wow. Which Symphony are you running ?

Member

nitriques commented Feb 20, 2015

Wow. Which Symphony are you running ?

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques
Member

nitriques commented Feb 20, 2015

@wdebusschere

This comment has been minimized.

Show comment
Hide comment
@wdebusschere

wdebusschere Feb 20, 2015

NULL.. version 2.5.3 updated last night, but seems that i had this problem already in a previous project with an older symphony version also..

wdebusschere commented Feb 20, 2015

NULL.. version 2.5.3 updated last night, but seems that i had this problem already in a previous project with an older symphony version also..

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

Using the current integration branch, I get a 404 page if the first of my params is 0!

Member

michael-e commented Feb 20, 2015

Using the current integration branch, I get a 404 page if the first of my params is 0!

@wdebusschere

This comment has been minimized.

Show comment
Hide comment
@wdebusschere

wdebusschere Feb 20, 2015

Maybe your datasource is configured to redirect to 404 when no results with filtering param

wdebusschere commented Feb 20, 2015

Maybe your datasource is configured to redirect to 404 when no results with filtering param

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

No, I am pretty sure that I removed all datasources.

  • example.com/page/1/1/ -- works
  • example.com/page/1/0/ -- works
  • example.com/page/hello/1/ -- works
  • example.com/page/0/1/ -- FAILS (404 page)
Member

michael-e commented Feb 20, 2015

No, I am pretty sure that I removed all datasources.

  • example.com/page/1/1/ -- works
  • example.com/page/1/0/ -- works
  • example.com/page/hello/1/ -- works
  • example.com/page/0/1/ -- FAILS (404 page)
@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Wow x2 then!

example.com/page/1/0/ -- works

Do you have the page param in the xml ?

Member

nitriques commented Feb 20, 2015

Wow x2 then!

example.com/page/1/0/ -- works

Do you have the page param in the xml ?

@nitriques nitriques changed the title from page param 0 gets ignored to When page param == 0, it gets ignored Feb 20, 2015

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

Indeed the page is called "test", and I have:

<current-page>test</current-page>
Member

michael-e commented Feb 20, 2015

Indeed the page is called "test", and I have:

<current-page>test</current-page>
@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member
  • example.com/page/99999999999999/1/ -- works

So it can not be caused by any datasource.

Member

michael-e commented Feb 20, 2015

  • example.com/page/99999999999999/1/ -- works

So it can not be caused by any datasource.

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

@wdebusschere stupid me. The var_dump we are looking for is var_dump($this->_param)die; (notice the added _)

Sorry, :(. Can you re-try ?? :(

Member

nitriques commented Feb 20, 2015

@wdebusschere stupid me. The var_dump we are looking for is var_dump($this->_param)die; (notice the added _)

Sorry, :(. Can you re-try ?? :(

@wdebusschere

This comment has been minimized.

Show comment
Hide comment
@wdebusschere

wdebusschere Feb 20, 2015

Output:
"page" ["param1"]=> string(3) "444" ["param2"]=> NULL

wdebusschere commented Feb 20, 2015

Output:
"page" ["param1"]=> string(3) "444" ["param2"]=> NULL

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

And param2 is the one == 0 I guess ?

Member

nitriques commented Feb 20, 2015

And param2 is the one == 0 I guess ?

@wdebusschere

This comment has been minimized.

Show comment
Hide comment
@wdebusschere

wdebusschere Feb 20, 2015

NO, param1 should be 0, param2 444
my url www.mysite.com/page/0/444/

wdebusschere commented Feb 20, 2015

NO, param1 should be 0, param2 444
my url www.mysite.com/page/0/444/

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Can you try to replace the empty call here by strlen($page_extra_bits) > 0 ?

Member

nitriques commented Feb 20, 2015

Can you try to replace the empty call here by strlen($page_extra_bits) > 0 ?

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Because empty('0') === true

See: http://php.net/manual/en/function.empty.php

Return Values

Returns FALSE if var exists and has a non-empty, non-zero value. Otherwise returns TRUE.

The following things are considered to be empty:

"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)
$var; (a variable declared, but without a value)

Member

nitriques commented Feb 20, 2015

Because empty('0') === true

See: http://php.net/manual/en/function.empty.php

Return Values

Returns FALSE if var exists and has a non-empty, non-zero value. Otherwise returns TRUE.

The following things are considered to be empty:

"" (an empty string)
0 (0 as an integer)
0.0 (0 as a float)
"0" (0 as a string)
NULL
FALSE
array() (an empty array)
$var; (a variable declared, but without a value)

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

Nope, that doesn't work if you have multiple params:

Symphony Warning: strlen() expects parameter 1 to be string, array given

Member

michael-e commented Feb 20, 2015

Nope, that doesn't work if you have multiple params:

Symphony Warning: strlen() expects parameter 1 to be string, array given

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Damn.

Member

nitriques commented Feb 20, 2015

Damn.

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

I have the feeling that things go wrong much earlier. Investigating…

Member

michael-e commented Feb 20, 2015

I have the feeling that things go wrong much earlier. Investigating…

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Thanks Michael, I really feel like this is bug... And pretty sure it has to do with a empty call.

Member

nitriques commented Feb 20, 2015

Thanks Michael, I really feel like this is bug... And pretty sure it has to do with a empty call.

@nitriques nitriques reopened this Feb 20, 2015

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

OK, the first thing that I found out is: Resolving the path/handle/params/extra_bits (and the like) is pretty adventurous. :-)

If you add:

var_dump($page_extra_bits); die();

to this line, then call a page like /test/1/1/, you get:

array(2) {
  [0]=>
  string(1) "1"
  [1]=>
  string(1) "1"
}

If your URL is like /test/1/0/:

array(2) {
  [0]=>
  string(1) "0"
  [1]=>
  string(1) "1"
}

BUT if it is /test/0/1/, you see:

array(1) {
  [0]=>
  string(1) "1"
}

Must have to do with array_popping. :-))

Member

michael-e commented Feb 20, 2015

OK, the first thing that I found out is: Resolving the path/handle/params/extra_bits (and the like) is pretty adventurous. :-)

If you add:

var_dump($page_extra_bits); die();

to this line, then call a page like /test/1/1/, you get:

array(2) {
  [0]=>
  string(1) "1"
  [1]=>
  string(1) "1"
}

If your URL is like /test/1/0/:

array(2) {
  [0]=>
  string(1) "0"
  [1]=>
  string(1) "1"
}

BUT if it is /test/0/1/, you see:

array(1) {
  [0]=>
  string(1) "1"
}

Must have to do with array_popping. :-))

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Must have to do with array_popping. :-))

Looks like it! or maybe array_filter or array_map

Member

nitriques commented Feb 20, 2015

Must have to do with array_popping. :-))

Looks like it! or maybe array_filter or array_map

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e
Member

michael-e commented Feb 20, 2015

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

But as I said, it looks a bit adventurous. Fixing this is a job for a brave man.

Member

michael-e commented Feb 20, 2015

But as I said, it looks a bit adventurous. Fixing this is a job for a brave man.

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

I am pretty sure that both phenomenons (omitting the param or throwing a 404) are caused by the above misbehaviour. Probably it's just the 404 logic that has been changed a bit. (I remember us working on that from time to time.)

Member

michael-e commented Feb 20, 2015

I am pretty sure that both phenomenons (omitting the param or throwing a 404) are caused by the above misbehaviour. Probably it's just the 404 logic that has been changed a bit. (I remember us working on that from time to time.)

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

Do you have a proposed fix ? 😄

Member

nitriques commented Feb 20, 2015

Do you have a proposed fix ? 😄

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

Nope, as I said, brave men only… What about you?

Member

michael-e commented Feb 20, 2015

Nope, as I said, brave men only… What about you?

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 20, 2015

Member

Honestly, I don't see the big picture in this code, but maybe @brendo can help?

Member

michael-e commented Feb 20, 2015

Honestly, I don't see the big picture in this code, but maybe @brendo can help?

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 20, 2015

Member

What about you?

Nope. And I am afraid to break something. I do hope @brendo feels like playing with 🔥

Member

nitriques commented Feb 20, 2015

What about you?

Nope. And I am afraid to break something. I do hope @brendo feels like playing with 🔥

brendo added a commit that referenced this issue Feb 22, 2015

@brendo

This comment has been minimized.

Show comment
Hide comment
@brendo

brendo Feb 22, 2015

Member

A nice example of where loose comparison can have adverse effects. Fixed!

Member

brendo commented Feb 22, 2015

A nice example of where loose comparison can have adverse effects. Fixed!

@brendo brendo self-assigned this Feb 22, 2015

@brendo brendo added this to the 2.6.0 milestone Feb 22, 2015

@michael-e

This comment has been minimized.

Show comment
Hide comment
@michael-e

michael-e Feb 22, 2015

Member

Confirmed, thanks!

Member

michael-e commented Feb 22, 2015

Confirmed, thanks!

@michael-e michael-e closed this Feb 22, 2015

@nitriques

This comment has been minimized.

Show comment
Hide comment
@nitriques

nitriques Feb 24, 2015

Member

Nice one Brendan! I knew you would like to play with 🔥

Member

nitriques commented Feb 24, 2015

Nice one Brendan! I knew you would like to play with 🔥

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