peachpiecompiler / peachpie

PeachPie - the PHP compiler and runtime for .NET and .NET Core
https://www.peachpie.io
Apache License 2.0
2.38k stars 203 forks source link

http_build_query() does not work properly #193

Closed gordon-matt closed 6 years ago

gordon-matt commented 6 years ago

As noted in this comment on issue #185, there is a problem with query string generation. I have tracked down the problem to be http_build_query(). Basically, you are missing ampersands between the parameters and for empty strings, you should set the value to "" instead of nothing.

jakubmisek commented 6 years ago

@gordon-matt nice, can you please post a sample test case which returns different results on PHP and PeachPie? Thank you

gordon-matt commented 6 years ago

@jakubmisek

Take a look at this working GET Request from an MVC5 site of mine that also uses this file manager (with Phalanger): ?editor=&type=0&lang=en_EN&popup=&crossdomain=&extensions=""&field_id=&relative_url=&akey=key&fldr=Folder1%2F&58D2D89945E2F

Now compare that with what is generated by Peachpie:

?editor=type=lang=en_ENpopup=crossdomain=field_id=relative_url=akey=keyfldr=Folder1%2F&58CD5BB1EA1F9

I debugged the code in question and saw the faulty query string returned from http_build_query()

The PHP code in the file manager is as follows:

$get_params = array(
    'editor'    => $editor,
    'type'      => $type_param,
    'lang'      => $lang,
    'popup'     => $popup,
    'crossdomain' => $crossdomain,
    'field_id'  => $field_id,
    'relative_url' => $return_relative_url,
    'akey'      => (isset($_GET['akey']) && $_GET['akey'] != '' ? $_GET['akey'] : 'key')
);
if(isset($_GET['CKEditorFuncNum'])){
    $get_params['CKEditorFuncNum'] = $_GET['CKEditorFuncNum'];
    $get_params['CKEditor'] = (isset($_GET['CKEditor'])? $_GET['CKEditor'] : '');
}
$get_params['fldr'] ='';

$get_params = http_build_query($get_params); // <-- Problem at this point

When debugging, I could see the value of $get_params was wrong after the call to http_build_query().

jakubmisek commented 6 years ago

@gordon-matt thanks, can you please post just a test case (working as it is) which we can run (standalone) on regular PHP and PeachPie.

gordon-matt commented 6 years ago

Well that would be easier for you than me; I'm not familiar with PHP... but surely it should work with any values at all, right? Just pass in an array of various strings and integers and see what happens. I highly doubt you need the exact same values I am passing in here.

jakubmisek commented 6 years ago

Test case:

print_r( http_build_query(["a" => 1, "b" => 2]) );

Expected: a=1&b=2 Actual: a=1b=2

gordon-matt commented 6 years ago

Excellent. Thanks for fixing the missing ampersands. I'll test as soon as you release on Nuget and report back

jakubmisek commented 6 years ago

@gordon-matt no problem, thanks for reporting this. I guess this could break a lot of existing PHP code ..

gordon-matt commented 6 years ago

You're welcome - I may not be familiar with PHP myself, but I am more than happy to help this project in any way I can (which seems to be with finding bugs :-D )...

gordon-matt commented 6 years ago

FYI - I just tested it and it works now! Getting very close to having a working file manager now!

jakubmisek commented 6 years ago

excellent