Skip to content

Commit f25bc86

Browse files
committed
fix(HyperRequest): Avoid double encoding using cfhttpparam
Lucee automatically encodes the `url` attribute. This led to double-encoding. Instead, we use the `cfhttpparam` without doing any encoding ourselves since the `cfhttpparam` always encodes.
1 parent 8336415 commit f25bc86

File tree

2 files changed

+40
-8
lines changed

2 files changed

+40
-8
lines changed

models/HyperRequest.cfc

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -422,8 +422,9 @@ component accessors="true" {
422422
*
423423
* @returns The full url for the request.
424424
*/
425-
function getFullUrl() {
426-
return getBaseUrl() & getUrl() & serializeQueryParams();
425+
function getFullUrl( withQueryString = false) {
426+
return getBaseUrl() & getUrl() &
427+
( arguments.withQueryString ? serializeQueryParams() : "" );
427428
}
428429

429430
/**
@@ -504,8 +505,8 @@ component accessors="true" {
504505
var queryString = listRest( arguments.url, "?" );
505506
var queryParams = listToArray( queryString, "&" );
506507
for ( var paramString in queryParams ) {
507-
var name = listFirst( paramString, "=" );
508-
var value = listRest( paramString, "=" );
508+
var name = decodeFromUrl( listFirst( paramString, "=" ) );
509+
var value = decodeFromUrl( listRest( paramString, "=" ) );
509510
setQueryParam( name, value );
510511
}
511512
return listFirst( arguments.url, "?" );
@@ -526,7 +527,9 @@ component accessors="true" {
526527
}
527528
queryParamNames.sort( "textnocase" );
528529
return "?" & queryParamNames.map( function( name ) {
529-
return "#encodeForURL( name )#=#encodeForURL( variables.queryParams[ name ] )#";
530+
return len( variables.queryParams[ name ] ) ?
531+
"#encodeForURL( name )#=#encodeForURL( variables.queryParams[ name ] )#" :
532+
"#encodeForURL( name )#";
530533
} ).toList( "&" );
531534
}
532535

@@ -600,6 +603,14 @@ component accessors="true" {
600603
);
601604
}
602605

606+
for ( var name in variables.queryParams ) {
607+
cfhttpparam(
608+
type = "url",
609+
name = name,
610+
value = variables.queryParams[ name ]
611+
);
612+
}
613+
603614
if ( hasBody() ) {
604615
if ( getBodyFormat() == "json" ) {
605616
cfhttpparam( type = "body", value = prepareBody() );

tests/specs/integration/GetSpec.cfc

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ component extends="tests.resources.ModuleIntegrationSpec" appMapping="/app" {
3434
var res = hyper.get( "https://jsonplaceholder.typicode.com/posts", {
3535
"userId" = 1
3636
} );
37-
expect( res.getRequest().getFullUrl() ).toBeWithCase( "https://jsonplaceholder.typicode.com/posts?userId=1" );
37+
expect( res.getRequest().getFullUrl( withQueryString = true ) )
38+
.toBeWithCase( "https://jsonplaceholder.typicode.com/posts?userId=1" );
3839
} );
3940

4041
it( "serializes query string params in the long hand", function() {
@@ -45,7 +46,26 @@ component extends="tests.resources.ModuleIntegrationSpec" appMapping="/app" {
4546
"userId" = 1
4647
} )
4748
.get();
48-
expect( res.getRequest().getFullUrl() ).toBeWithCase( "https://jsonplaceholder.typicode.com/posts?userId=1" );
49+
expect( res.getRequest().getFullUrl( withQueryString = true) )
50+
.toBeWithCase( "https://jsonplaceholder.typicode.com/posts?userId=1" );
51+
} );
52+
53+
it( "deserializes query string parameters in the url (to reserialize later)", function() {
54+
var res = hyper
55+
.setBaseUrl( "https://jsonplaceholder.typicode.com" )
56+
.setUrl( "/posts?param=with+spaces" )
57+
.get();
58+
expect( res.getRequest().getFullUrl( withQueryString = true ) )
59+
.toBeWithCase( "https://jsonplaceholder.typicode.com/posts?param=with+spaces" );
60+
} );
61+
62+
it( "can handle params with no value in the url", function() {
63+
var res = hyper
64+
.setBaseUrl( "https://jsonplaceholder.typicode.com" )
65+
.setUrl( "/posts?flag" )
66+
.get();
67+
expect( res.getRequest().getFullUrl( withQueryString = true ) )
68+
.toBeWithCase( "https://jsonplaceholder.typicode.com/posts?flag" );
4969
} );
5070

5171
it( "combines both query params and the query string in the url", function() {
@@ -56,7 +76,8 @@ component extends="tests.resources.ModuleIntegrationSpec" appMapping="/app" {
5676
"force" = "true"
5777
} )
5878
.get();
59-
expect( res.getRequest().getFullUrl() ).toBeWithCase( "https://jsonplaceholder.typicode.com/posts?force=true&fwreinit=true&userId=1" );
79+
expect( res.getRequest().getFullUrl( withQueryString = true ) )
80+
.toBeWithCase( "https://jsonplaceholder.typicode.com/posts?force=true&fwreinit=true&userId=1" );
6081
} );
6182

6283
it( "has access to the original HyperRequest in the HyperResponse", function() {

0 commit comments

Comments
 (0)