fix: some options were not allowing to be set to null

Author: JCMais

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed  
 - Fix `curly.post` and `curly.head` using wrong libcurl options to set the HTTP Method.
 - Fix `postinstall` script not working properly.
+- Setting the `HTTPPOST` option to `null`would, wrongly, throw an Error.
+- Setting any string option to `null` would, wrongly, throw an Error.
 ### Added
 - Added back prebuilt binaries for:
   - Electron v3, v4 and v5

src/Easy.cc

@@ -430,7 +430,8 @@ size_t Easy::OnData(char* data, size_t size, size_t nmemb) {
   int32_t returnValue = -1;
 
   const int argc = 3;
-  v8::Local<v8::Object> buf = Nan::CopyBuffer(data, static_cast<uint32_t>(dataLength)).ToLocalChecked();
+  v8::Local<v8::Object> buf =
+      Nan::CopyBuffer(data, static_cast<uint32_t>(dataLength)).ToLocalChecked();
   v8::Local<v8::Uint32> sizeArg = Nan::New<v8::Uint32>(static_cast<uint32_t>(size));
   v8::Local<v8::Uint32> nmembArg = Nan::New<v8::Uint32>(static_cast<uint32_t>(nmemb));
 
@@ -485,7 +486,8 @@ size_t Easy::OnHeader(char* data, size_t size, size_t nmemb) {
   int32_t returnValue = -1;
 
   const int argc = 3;
-  v8::Local<v8::Object> buf = Nan::CopyBuffer(data, static_cast<uint32_t>(dataLength)).ToLocalChecked();
+  v8::Local<v8::Object> buf =
+      Nan::CopyBuffer(data, static_cast<uint32_t>(dataLength)).ToLocalChecked();
   v8::Local<v8::Uint32> sizeArg = Nan::New<v8::Uint32>(static_cast<uint32_t>(size));
   v8::Local<v8::Uint32> nmembArg = Nan::New<v8::Uint32>(static_cast<uint32_t>(nmemb));
 
@@ -1107,8 +1109,11 @@ NAN_METHOD(Easy::SetOpt) {
     }
     // linked list options
   } else if ((optionId = IsInsideCurlConstantStruct(curlOptionLinkedList, opt))) {
-    // HTTPPOST is a special case, since it's an array of objects.
-    if (optionId == CURLOPT_HTTPPOST) {
+    if (value->IsNull()) {
+      setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), NULL);
+
+      // HTTPPOST is a special case, since it's an array of objects.
+    } else if (optionId == CURLOPT_HTTPPOST) {
       std::string invalidArrayMsg = "HTTPPOST option value should be an Array of Objects.";
 
       if (!value->IsArray()) {
@@ -1258,66 +1263,57 @@ NAN_METHOD(Easy::SetOpt) {
       }
 
     } else {
-      if (value->IsNull()) {
-        setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), NULL);
-
-      } else {
-        if (!value->IsArray()) {
-          Nan::ThrowTypeError("Option value must be an Array.");
-          return;
-        }
+      if (!value->IsArray()) {
+        Nan::ThrowTypeError("Option value must be an Array.");
+        return;
+      }
 
-        // convert value to curl linked list (curl_slist)
-        curl_slist* slist = NULL;
-        v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(value);
+      // convert value to curl linked list (curl_slist)
+      curl_slist* slist = NULL;
+      v8::Local<v8::Array> array = v8::Local<v8::Array>::Cast(value);
 
-        for (uint32_t i = 0, len = array->Length(); i < len; ++i) {
-          slist = curl_slist_append(slist, *Nan::Utf8String(Nan::Get(array, i).ToLocalChecked()));
-        }
+      for (uint32_t i = 0, len = array->Length(); i < len; ++i) {
+        slist = curl_slist_append(slist, *Nan::Utf8String(Nan::Get(array, i).ToLocalChecked()));
+      }
 
-        setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), slist);
+      setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), slist);
 
-        if (setOptRetCode == CURLE_OK) {
-          obj->toFree->slist.push_back(slist);
-        }
+      if (setOptRetCode == CURLE_OK) {
+        obj->toFree->slist.push_back(slist);
       }
     }
-
     // check if option is string, and the value is correct
   } else if ((optionId = IsInsideCurlConstantStruct(curlOptionString, opt))) {
-    if (!value->IsString()) {
-      Nan::ThrowTypeError("Option value must be a string.");
-      return;
-    }
-
-    // Create a string copy
-    bool isNull = value->IsNull();
-
-    if (isNull) {
+    if (value->IsNull()) {
       setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), NULL);
-    }
+    } else {
+      if (!value->IsString()) {
+        Nan::ThrowTypeError("Option value must be a string.");
+        return;
+      }
 
-    Nan::Utf8String value(info[1]);
+      Nan::Utf8String value(info[1]);
 
-    size_t length = static_cast<size_t>(value.length());
+      size_t length = static_cast<size_t>(value.length());
 
-    std::string valueStr = std::string(*value, length);
+      std::string valueStr = std::string(*value, length);
 
-    // libcurl makes a copy of the strings after version 7.17, CURLOPT_POSTFIELD
-    // is the only exception
-    if (static_cast<CURLoption>(optionId) == CURLOPT_POSTFIELDS) {
-      std::vector<char> valueChar = std::vector<char>(valueStr.begin(), valueStr.end());
-      valueChar.push_back(0);
+      // libcurl makes a copy of the strings after version 7.17, CURLOPT_POSTFIELD
+      // is the only exception
+      if (static_cast<CURLoption>(optionId) == CURLOPT_POSTFIELDS) {
+        std::vector<char> valueChar = std::vector<char>(valueStr.begin(), valueStr.end());
+        valueChar.push_back(0);
 
-      setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), &valueChar[0]);
+        setOptRetCode = curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), &valueChar[0]);
 
-      if (setOptRetCode == CURLE_OK) {
-        obj->toFree->str.push_back(std::move(valueChar));
-      }
+        if (setOptRetCode == CURLE_OK) {
+          obj->toFree->str.push_back(std::move(valueChar));
+        }
 
-    } else {
-      setOptRetCode =
-          curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), valueStr.c_str());
+      } else {
+        setOptRetCode =
+            curl_easy_setopt(obj->ch, static_cast<CURLoption>(optionId), valueStr.c_str());
+      }
     }
 
     // check if option is an integer, and the value is correct

test/curl/setOpt.spec.ts

@@ -40,6 +40,23 @@ describe('setOpt()', () => {
     curl.setOpt(Curl.option.URL, url)
   })
 
+  it('should be able to set string value back to null', () => {
+    curl.setOpt('URL', url)
+    curl.setOpt('URL', null)
+  })
+
+  it('should be able to set integer value back to null', () => {
+    curl.setOpt('ACCEPTTIMEOUT_MS', 30000)
+    curl.setOpt('ACCEPTTIMEOUT_MS', null)
+  })
+
+  it('should be able to set function value back to null', () => {
+    curl.setOpt('WRITEFUNCTION', () => {
+      return 0
+    })
+    curl.setOpt('WRITEFUNCTION', null)
+  })
+
   it('should not accept invalid argument type', () => {
     const optionsToTest = [
       ['URL', 0],
@@ -119,6 +136,26 @@ describe('setOpt()', () => {
   })
 
   describe('HTTPPOST', () => {
+    it('should work', () => {
+      curl.setOpt('HTTPPOST', [
+        {
+          name: 'field',
+          contents: 'value',
+        },
+      ])
+    })
+
+    it('should be able to set option back to null', () => {
+      curl.setOpt('HTTPPOST', [
+        {
+          name: 'field',
+          contents: 'value',
+        },
+      ])
+
+      curl.setOpt('HTTPPOST', null)
+    })
+
     it('should not accept invalid arrays', () => {
       try {
         // @ts-ignore