Http: Null request payload/body values not handled properly

Created on 6 Apr 2017  路  5Comments  路  Source: dart-lang/http

It appears that http/src/utils.dart's mapToQuery() method doesn't properly handle objects with null values causing unhandled exceptions:

Unhandled exception:
NoSuchMethodError: The getter 'length' was called on null.
Receiver: null
Tried calling: length
...
#3      Uri.encodeQueryComponent (dart:core/uri.dart:1055)
...

Repro code:

import 'package:http/http.dart' as http;

main(List<String> args) async {
  // ======== Works
  try {
    // -- load model
    var foo = loadExistingModel("123");
    // -- modify
    foo.message += " modified";
    // -- save
    var postResult = await http.put('http://www.example.com/myModel', body: foo.asMap);
    print("++update succeeded");

  } catch(e) {
    print("!!update failed");
    rethrow;
  }

  // ======== Broken (due to `null` value for `id` property)
  try {
    // -- create model
    var foo = new MyModel("brand new model");
    // -- save new
    var postResult = await http.post('http://www.example.com/myModel', body: foo.asMap);
    print("++create succeeded");

  } catch(e) {
    print("!!create failed");
    rethrow;
  }
}

class MyModel {
  final String id; // -- only populated once saved
  String message;

  MyModel(this.message, [this.id]);

  Map<String, dynamic> get asMap {
    return {
      "id"     : id,
      "message": message
    };
  }
}

loadExistingModel(String id) {
  // fetch model by id
  // getResult = await http.get('http://www.example.com/myModel/$id');
  return new MyModel("existing model", id);
}

Suggested fix:

http-0.11.3+9/lib/src/utils.dart
 String mapToQuery(Map<String, String> map, {Encoding encoding}) {
   var pairs = <List<String>>[];
   map.forEach((key, value) =>
+      if (value == null) {
+        return; // -- skip null elements
+      }
+
       pairs.add([Uri.encodeQueryComponent(key, encoding: encoding),
                  Uri.encodeQueryComponent(value, encoding: encoding)]));
+  }
   return pairs.map((pair) => "${pair[0]}=${pair[1]}").join("&");
 }

Alternative fix:

http-0.11.3+9/lib/src/utils.dart
 String mapToQuery(Map<String, String> map, {Encoding encoding}) {
+  map.keys
+      .where((k) => (map[k] == null)).toList() // -- keys for null elements
+      .forEach(map.remove);
+
   var pairs = <List<String>>[];
   map.forEach((key, value) =>
       pairs.add([Uri.encodeQueryComponent(key, encoding: encoding),
                  Uri.encodeQueryComponent(value, encoding: encoding)]));

   return pairs.map((pair) => "${pair[0]}=${pair[1]}").join("&");
 }
closed-as-intended

Most helpful comment

In that case, I feel like we could benefit from a better error message. Maybe something like this?

http-0.11.3+9/lib/src/utils.dart
 String mapToQuery(Map<String, String> map, {Encoding encoding}) {
+  if (map.values.any((v) => (v == null))) { // -- null values found
+      throw new ArgumentError(map, "map", "NULL values not supported in querystring");
+  }
+
   var pairs = <List<String>>[];
   map.forEach((key, value) =>
       pairs.add([Uri.encodeQueryComponent(key, encoding: encoding),
                  Uri.encodeQueryComponent(value, encoding: encoding)]));

   return pairs.map((pair) => "${pair[0]}=${pair[1]}").join("&");
 }

All 5 comments

This behavior is intentional. It's generally the case in Dart that null values aren't allowed unless it's specifically indicated that they are鈥攐nce we have support for explicitly declaring the nullability of a type, this will have non-nullable string values. It's the caller's responsibility to make sure that nulls don't get passed where they don't belong, not the callee's responsibility to clean them up.

In that case, I feel like we could benefit from a better error message. Maybe something like this?

http-0.11.3+9/lib/src/utils.dart
 String mapToQuery(Map<String, String> map, {Encoding encoding}) {
+  if (map.values.any((v) => (v == null))) { // -- null values found
+      throw new ArgumentError(map, "map", "NULL values not supported in querystring");
+  }
+
   var pairs = <List<String>>[];
   map.forEach((key, value) =>
       pairs.add([Uri.encodeQueryComponent(key, encoding: encoding),
                  Uri.encodeQueryComponent(value, encoding: encoding)]));

   return pairs.map((pair) => "${pair[0]}=${pair[1]}").join("&");
 }

I definitely appreciate the principles that @nex3 mentions and agree that non-nullable types will help clarify this in the future.
However without a specific error, such as @KOGI suggests, this looks like an implementation mistake on the part of the library author.

For the same reason, we generally don't include explicit null checks in places where null isn't expected鈥攐therwise we'd have them all over the place.

Hi, so if I need to update a record on the server and set the value of one of the fields to be null, how can I accomplish this?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

IrosTheBeggar picture IrosTheBeggar  路  5Comments

Blykam picture Blykam  路  6Comments

jet10000 picture jet10000  路  5Comments

PieterAelse picture PieterAelse  路  4Comments

HaiVu5583 picture HaiVu5583  路  3Comments