Created by: samccone
Activity
155 155 156 156 // Only join the query string if it exists so we don't have trailing a '?' 157 157 // on every request 158 lastSegs[1] && retSegs.push(lastSegs[1]); 158 159 // Handle case where there could be multiple ? in the URL. 160 retSegs.concat(lastSegs); 155 155 156 156 // Only join the query string if it exists so we don't have trailing a '?' 157 157 // on every request 158 lastSegs[1] && retSegs.push(lastSegs[1]); 158 159 // Handle case where there could be multiple ? in the URL. 160 retSegs.concat(lastSegs); Created by: koolc
Do you confirm the code is correct? o?o
I think, the code
retSegs.concat(lastSegs)
doesn't seemingly change the arrayretSegs
value when it doesn't assigned toretSegs
likeretSegs = retSegs.concat(lastSegs);
, so wouldn't theretSegs
value in the last statementreturn retSegs.join('?')
be original? You can try by a multiple '?' urls.The codes recommended:
retSegs.push.apply(retSegs, lastSegs); return retSegs.join('?');
or
return retSegs.concat(lastSegs).join('?')
mentioned in merge request !744 (closed)
155 155 156 156 // Only join the query string if it exists so we don't have trailing a '?' 157 157 // on every request 158 lastSegs[1] && retSegs.push(lastSegs[1]); 158 159 // Handle case where there could be multiple ? in the URL. 160 retSegs.concat(lastSegs); 155 155 156 156 // Only join the query string if it exists so we don't have trailing a '?' 157 157 // on every request 158 lastSegs[1] && retSegs.push(lastSegs[1]); 158 159 // Handle case where there could be multiple ? in the URL. 160 retSegs.concat(lastSegs); Created by: koolc
To use 'concat' can create a new array indeed, but why to need a new array? And that, the new array isn't used by you to the codes after it. So, the variable
retSegs
value in return statement is still the old value, that is to say, the array length of the variableretSegs
is still 1.If you want to use 'concat' function, an assignment statement is required, like
retSegs = retSegs.concat(lastSegs)
, because of the example below.var a = [1, 2]; var b = a.concat([3,4]); // to assign the variable 'b'. console.log(a, b) // output: [1, 2] [1, 2, 3, 4] var c = a.join('-'); // ! This is your case: 'a' doesn't been changed and is still old value. var d = b.join('-'); // ! This is my meaning: new elements have been added into 'b'. console.log(c, d); // output: 1-2 1-2-3-4 /* expected result: 1-2-3-4, instead of: 1-2 */
So, I think that the code
retSegs.concat(lastSegs)
should be changed toretSegs = retSegs.concat(lastSegs)
. Do you?
155 155 156 156 // Only join the query string if it exists so we don't have trailing a '?' 157 157 // on every request 158 lastSegs[1] && retSegs.push(lastSegs[1]); 158 159 // Handle case where there could be multiple ? in the URL. 160 retSegs.concat(lastSegs); 155 155 156 156 // Only join the query string if it exists so we don't have trailing a '?' 157 157 // on every request 158 lastSegs[1] && retSegs.push(lastSegs[1]); 158 159 // Handle case where there could be multiple ? in the URL. 160 retSegs.concat(lastSegs); Created by: koolc
OK, I see now, and the codes in master are already right. I just had doubts when I saw the codes in the branch http-proxy/common.js#L160 from #748 are below:
// Handle case where there could be multiple ? in the URL. retSegs.concat(lastSegs); return retSegs.join('?')
So sorry! ^-^!