angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
96.23k stars 25.5k forks source link

HttpRequest.clone is duplicating query parameters (type 'append') #18812

Closed IsNull closed 5 years ago

IsNull commented 7 years ago

I'm submitting a...


[ ] Regression (a behaviour that used to work and stopped working in a new release)
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behaviour

Using the new HttpClient HttpRequest.clone() duplicates existing query parameters

As an example, the following interceptor simply adds a query param. As an unwanted side effect of the clone method, existing query params get duplicated:

    intercept(
        req: HttpRequest<any>,
        next: HttpHandler
    ): Observable<HttpEvent<any>> {

            req = req.clone({
                setParams: {
                    locale: this.translate.currentLang
                }
            });
        return next.handle(req);
    }

In essence: ?sort=id,asc becomes : ?sort=id,asc&sort=id,asc&locale=en instead of ?sort=id,asc&locale=en when the query gets executed.

Expected behavior

HttpRequest.clone() should not duplicate existing query parameters in the request.

req = req.clone({
                setParams: {
                    locale: this.translate.currentLang
                }
            });

Note: The query parameters where added using HttpParams.append(...) as there indeed could exist multiple query params, but of course not with the same content.

Minimal reproduction of the problem with instructions

    let params = new HttpParams();

    params = params.append('sort', 'id,asc');

    let req2 = new HttpRequest('GET', 'url', null, {
        params: params
    });
    let clone = req2.clone({
        setParams: {
            locale: 'de'
        }
    });
    this.dump = JSON.stringify(clone.params);

See this plunker: https://plnkr.co/edit/mqJCe7QbUWpzWQNbeOJS?p=preview

What is the motivation / use case for changing the behaviour?

Duplicate query params cause errors in the backend.

Environment

Angular version: 4.3.5

Browser:

Seems not to be browser related.

For Tooling issues:

Others:

IsNull commented 7 years ago

I have added a plunker which demonstrates the issue.

ism-ssw commented 7 years ago

I have found a similar issue but with append on its own when trying to add an array for a parameter. If I use (where state is an array of strings)

let params:HttpParams = new HttpParams();
let state = ["one","two"];
state.forEach( (s,ix) => { params.append('state',s);})

// produces &state=one&state=two&state=one&state=two
state.forEach( (s,ix) => {params = ix==0?params.set('state',s):params.append('state',s);}
//produces &state=one&state=two
bsh314 commented 7 years ago

Same here. `l et params = req.params; let headers = req.headers;

if (this.ts.currentLang && !params.get('lang')) {
  params = params.append('lang', this.ts.currentLang);
}

if (this.ss.token) {
  headers = headers.append('Authorization', `Bearer ${this.ss.token}`);
}

return next.handle(req.clone({
  params: params,
  headers: headers,
  withCredentials: true,
}));

`

mehtadata commented 7 years ago

This is also happening for us. Seems to be an issue:

  1. When you supply a multiple values with the same key e.g. url?key=val1&key=val2

  2. When you do the same with comma syntax: url?key=val1,val2

CurtisL commented 7 years ago

Figured out a lightweight workaround. Instead of using setParams: just replace the params entirely with params: which accepts HttpParams. (Reference)

intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
  // Keeps the original request params. as a new HttpParams
  let newParams = new HttpParams({fromString: request.params.toString()});

  // Add any params (can also chain .append() but I was conditionally adding params)
  newParams = newParams.append('new_param', 'param_value');
  newParams = newParams.append('another_param', 'another_value');

  // Clone the request with params instead of setParams
  const requestClone = request.clone({
    params: newParams
  });

  return next.handle(requestClone);
}

Hope this helps anyone who comes across this until there is a fix.

rfuhrer commented 6 years ago

I think this is related to the bug #20430 and will hopefully be fixed by my PR #20610 .

It duplicates all your HttpParams if you call any of the 'read' functions more than once which I believe would happen if you clone a request

LucaColombi commented 6 years ago

I actually still have this, is the fix relased?

jpmckearin commented 6 years ago

@LucaColombi in the comment just above yours, @rfuhrer said he has made a PR #20610. If you click on that link, you will notice the PR is still "Open". Also, you'll notice this issue is still "Open" and that ngbot added this to "Backlog" and "needsTriage" 3 days before your comment. Therefore, I believe this issue has not been fixed. In the meantime, @CurtisL posted 2 comments ago with an easy, temporary work-around.

alexxxnf commented 6 years ago

Hi guys!

I’d like to thank @CurtisL for his solution. It worked for me until I started using custom encoder.

Can anyone suggest a workaround that allows to clone HttpParams along with custom encoder?

msorens commented 5 years ago

FYI I am not using a custom encoder but the solution from @CurtisL still does not work for me. Not sure what is unique in my environment. So this is a blocker for me.

rfuhrer commented 5 years ago

@msorens They finally decided to fix this in Angular 8: https://github.com/angular/angular/commit/ee65d0d4ab42344c117e7b2eb3e58585c8759cd9

So you should be good after you upgrade.

msorens commented 5 years ago

Thanks for that info @rfuhrer ! The upgrade was on my list; now I have more incentive.

IsNull commented 5 years ago

Thanks for the heads up @rfuhrer. This one took quite a while ^^

Closing this issue as fixed by ee65d0d4ab42344c117e7b2eb3e58585c8759cd9

Anj21 commented 5 years ago

Hello everyone, is this issue resolve, without upgrading to my project from Angular7 to Angular8. I am also facing the same issue. i used interceptor for access-token. Now in interceptor, i am adding common params, like access token, which require for every request, but when we API hits in which custom params are going, like id, name, date, It doubles the params. Can anyone help to sort out the issue. Thank you

appde commented 5 years ago

Make sure you enter your query parameters not as part of the url, but as options, i.e.:

  1. this.http.get<UserData>( `${environment.databaseURL}/users/${id}.json?auth=${token}` ); transforms into:

  2. this.http.get<UserData>( `${environment.databaseURL}/users/${id}.json`, { params: { auth: `${token}` } } );

In the first case, you will have a double query parameter 'auth' even after having this request.clone({ params: request.params.set('auth', `${token}`) }); somewhere in your interceptor.

Tested on "@angular/common": "8.2.5"

angular-automatic-lock-bot[bot] commented 5 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.