iron-ajax icon indicating copy to clipboard operation
iron-ajax copied to clipboard

_wwwFormUrlEncodePiece does not account for null and undefined values

Open tuespetre opened this issue 9 years ago • 3 comments

_wwwFormUrlEncodePiece does not account for null and undefined values.

Expected outcome

A null or undefined value would be coalesced to an empty string during encoding.

Actual outcome

An error is thrown.

Steps to reproduce

  1. Put an iron-ajax element in the page with a contentType of application/x-www-form-urlencoded
  2. Set its body to a POJO with at least one defined key whose value is null or undefined
  3. Attempt to invoke generateRequest() on the iron-ajax element

tuespetre avatar Feb 18 '16 18:02 tuespetre

jsbin to reproduce: https://jsbin.com/pibaregaze/edit?html,output

I'm not sure what the right way of handling this is though. I could see ignoring those key-value pairs, or encoding them as the empty string as you suggest. It's not obvious to me though that throwing an error isn't reasonable here though, as there's not a clear and unambiguous x-www-form-urlencoded encoding for null.

rictic avatar Feb 18 '16 19:02 rictic

@rictic true, very good points to consider. Of course for now I have taken to doing the coalescing before hitting iron-ajax. Maybe this is more of a documentation thing

tuespetre avatar Feb 18 '16 20:02 tuespetre

This was in February... I opened this today:

https://github.com/PolymerElements/iron-form/issues/153

Basically, a lot of elements will have undefined as value when they haven't been touched. Notable examples: paper-textarea, paper-dropdown-menu.

I can see that null is taken care of:

https://github.com/PolymerElements/iron-ajax/blob/c6c5c80fed433502eb270a9ef9c3854bc5892d22/iron-request.html#L428-L430

However, undefined is still an issue.

It looks like this fix:

https://github.com/PolymerElements/iron-ajax/pull/207

In response to:

https://github.com/PolymerElements/iron-ajax/issues/214

Didn't quite go far enough, and didn't include undefined?

mercmobily avatar Sep 28 '16 16:09 mercmobily