search

oracle / cordova-plugin-wkwebview-file-xhr

Cordova Plugin for WebView File XHR
Universal Permissive License v1.0
138 stars 120 forks source link

Casting non-string headers #13

Closed rdantonio closed 6 years ago

rdantonio commented 6 years ago

First wanted to thank you for the effort of this plugin, it's been making my transition off of UIWebView so much easier for my use cases.

I've noticed one different behavior between the way the normal XHR method works with headers though. Previously if we set header values by enumerating a JSON object and call: request.setRequestHeader(key, value);

If value is anything other than a string (in my case a number) the original XHR would cast it to a string and use it. It seems like this plugin throws out these values though when going through the header dictionary though:

if (![key isKindOfClass:NSString.class] || ![obj isKindOfClass:NSString.class]) return; [request setValue:(NSString *)obj forHTTPHeaderField:(NSString *)key];

Would it be possible to add casting for numbers? I don't know Objective-C too well yet but something like this?

NSString* strKey = ![key isKindOfClass:NSString.class] ? [key stringValue] : (NSString *)key; NSString* strValue = ![obj isKindOfClass:NSString.class] ? [obj stringValue] : (NSString *)obj; [request setValue:strValue forHTTPHeaderField:strKey];

Thank you!

gvanmat commented 6 years ago

Thanks for reporting the issue. I think we need to address this at the JavaScript level versus on the iOS side. We should be normalizing the header values.

You might not have access to the JS code setting the header request values but that might be a workaround until we can get this addressed.

request.setRequestHeader(key, "" + value);

rdantonio commented 6 years ago

I don't have access to this specific JS like you said but I was able to put something in place in the meantime that seems to be working.

Normalizing at the JS level sounds good as well, thank you for the quick response!

gvanmat commented 6 years ago

@rdantonio I pushed a fix into main. When you have time, please help me verify the fix.

cordova plugin remove cordova-plugin-wkwebview-file-xhr cordova plugin add https://github.com/oracle/cordova-plugin-wkwebview-file-xhr.git

rdantonio commented 6 years ago

Thank you! I ran a quick test and it's working for the case it was failing for previously. Looks good!