Skip to content

Commit 313fc87

Browse files
committed
Ensured that HTTPClient is reused unless a custom http.Transport is needed - also exposed an error in the URL query builder (internal API)
Signed-off-by: Levi Gross <[email protected]>
1 parent cf8f8ca commit 313fc87

File tree

3 files changed

+53
-9
lines changed

3 files changed

+53
-9
lines changed

base_get_test.go

+8
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,14 @@ func TestGetInvalidURL(t *testing.T) {
160160
}
161161
}
162162

163+
func TestGetInvalidURLNoParams(t *testing.T) {
164+
resp := <-GetAsync("%../dir/", nil)
165+
166+
if resp.Error == nil {
167+
t.Error("Some how the request was valid to make request", resp.Error)
168+
}
169+
}
170+
163171
func TestGetXMLSerialize(t *testing.T) {
164172
resp := <-GetAsync("http://httpbin.org/xml", nil)
165173

request.go

+27-6
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import (
99
"io"
1010
"mime"
1111
"mime/multipart"
12+
"net"
1213
"net/http"
1314
"net/url"
1415
"strings"
16+
"time"
1517
)
1618

1719
// RequestOptions is the location that of where the data
@@ -79,8 +81,15 @@ func buildRequest(httpMethod, url string, ro *RequestOptions) (*http.Response, e
7981
httpClient := buildHTTPClient(ro)
8082

8183
// Build our URL
84+
var err error
85+
8286
if ro.Params != nil {
83-
url = buildURLParams(url, ro.Params)
87+
url, err = buildURLParams(url, ro.Params)
88+
89+
if err != nil {
90+
return nil, err
91+
}
92+
8493
}
8594

8695
// Build the request
@@ -232,23 +241,35 @@ func encodePostValues(postValues map[string]string) string {
232241
}
233242

234243
func buildHTTPClient(ro *RequestOptions) *http.Client {
244+
if ro.InsecureSkipVerify != true && ro.DisableCompression != true {
245+
return http.DefaultClient
246+
}
247+
235248
return &http.Client{
236249
Transport: &http.Transport{
237-
DisableCompression: ro.DisableCompression,
250+
// These are borrowed from the default transporter
251+
Proxy: http.ProxyFromEnvironment,
252+
Dial: (&net.Dialer{
253+
Timeout: 30 * time.Second,
254+
KeepAlive: 30 * time.Second,
255+
}).Dial,
256+
TLSHandshakeTimeout: 10 * time.Second,
257+
258+
// He comes the user settings
238259
TLSClientConfig: &tls.Config{InsecureSkipVerify: ro.InsecureSkipVerify},
260+
DisableCompression: ro.DisableCompression,
239261
},
240262
}
241-
242263
}
243264

244265
// buildURLParams returns a URL with all of the params
245266
// Note: This function will override current URL params if they contradict what is provided in the map
246267
// That is what the "magic" is on the last line
247-
func buildURLParams(userURL string, params map[string]string) string {
268+
func buildURLParams(userURL string, params map[string]string) (string, error) {
248269
parsedURL, err := url.Parse(userURL)
249270

250271
if err != nil {
251-
return userURL
272+
return "", err
252273
}
253274

254275
parsedQuery, err := url.ParseQuery(parsedURL.RawQuery)
@@ -261,7 +282,7 @@ func buildURLParams(userURL string, params map[string]string) string {
261282
[]string{strings.Replace(parsedURL.String(),
262283
"?"+parsedURL.RawQuery, "", -1),
263284
parsedQuery.Encode()},
264-
"?")
285+
"?"), nil
265286
}
266287

267288
// addHTTPHeaders adds any additional HTTP headers that need to be added are added here including:

request_test.go

+18-3
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,36 @@ package grequests
33
import "testing"
44

55
func TestAddQueryStringParams(t *testing.T) {
6-
userURL := buildURLParams("https://www.google.com/", map[string]string{"1": "2", "3": "4"})
6+
userURL, err := buildURLParams("https://www.google.com/", map[string]string{"1": "2", "3": "4"})
7+
8+
if err != nil {
9+
t.Error("URL Parse Error: ", err)
10+
}
11+
712
if userURL != "https://www.google.com/?1=2&3=4" {
813
t.Error("URL params not properly built", userURL)
914
}
1015
}
1116

1217
func TestSortAddQueryStringParams(t *testing.T) {
13-
userURL := buildURLParams("https://www.google.com/", map[string]string{"3": "4", "1": "2"})
18+
userURL, err := buildURLParams("https://www.google.com/", map[string]string{"3": "4", "1": "2"})
19+
20+
if err != nil {
21+
t.Error("URL Parse Error: ", err)
22+
}
23+
1424
if userURL != "https://www.google.com/?1=2&3=4" {
1525
t.Error("URL params not properly built and sorted", userURL)
1626
}
1727
}
1828

1929
func TestAddQueryStringParamsExistingParam(t *testing.T) {
20-
userURL := buildURLParams("https://www.google.com/?5=6", map[string]string{"3": "4", "1": "2"})
30+
userURL, err := buildURLParams("https://www.google.com/?5=6", map[string]string{"3": "4", "1": "2"})
31+
32+
if err != nil {
33+
t.Error("URL Parse Error: ", err)
34+
}
35+
2136
if userURL != "https://www.google.com/?1=2&3=4&5=6" {
2237
t.Error("URL params not properly built and sorted", userURL)
2338
}

0 commit comments

Comments
 (0)