Skip to content

fixed an issue in checking https#46

Merged
ianpartridge merged 6 commits intoKitura:masterfrom
TheM4hd1:master
Dec 17, 2018
Merged

fixed an issue in checking https#46
ianpartridge merged 6 commits intoKitura:masterfrom
TheM4hd1:master

Conversation

@TheM4hd1
Copy link
Contributor

@TheM4hd1 TheM4hd1 commented Dec 11, 2018

replaced .contains("https") with .hasPrefix("https")
.contains can cause a problem.

@TheM4hd1
fixed an issue in checking https
d48ba94 
replaced .contains("https") with .hasPrefix("https")
.contains can cause a problem.
@CLAassistant
Copy link

CLAassistant commented Dec 11, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ianpartridge ianpartridge added this to the 2018.26 milestone Dec 12, 2018
@ianpartridge
Copy link
Contributor

ianpartridge commented Dec 12, 2018

Thanks for the PR! We'll get it reviewed soon :)

@nethraravindran
Copy link
Contributor

nethraravindran commented Dec 13, 2018 •
edited
Loading

@TheM4hd1 Thanks for the PR! Can we have a test case?

@TheM4hd1
Copy link
Contributor Author

TheM4hd1 commented Dec 13, 2018

tests were include a function named testGetSelfSignedCert, which tests ssl, the problem were in the initializing request which fixed with replacing .contains with .hasPrefix, anyway I've added a test again.
the test url is an insecure url but containing https which could causes error when requesting.
it fixes now.
Regards.

@nethraravindran
Copy link
Contributor

nethraravindran commented Dec 13, 2018

@TheM4hd1 can you please look into travis failures?

@TheM4hd1
Update SwiftyRequestTests.swift
d5ba798 
replaced test url, travis ip addresses were blocked by previous server.
@TheM4hd1
Update SwiftyRequestTests.swift
6e25b1e
@TheM4hd1
Copy link
Contributor Author

TheM4hd1 commented Dec 13, 2018

I'm getting crazy with this travis, thats not my test case error,
mine passed.

Test Case '-[SwiftyRequestTests.SwiftyRequestTests testInsecureConnection]' started.
Test Case '-[SwiftyRequestTests.SwiftyRequestTests testInsecureConnection]' passed (0.209 seconds).

@nethraravindran
Copy link
Contributor

nethraravindran commented Dec 14, 2018

@TheM4hd1 Can you try reproducing this issue on Linux or a docker. It can be a corelibs-foundation bug as well ( I doubt ). Please let me know what you think or if you need some help on this

@TheM4hd1
Copy link
Contributor Author

TheM4hd1 commented Dec 15, 2018

I've tested on ubuntu 16.04 and it works fine.
Test Results

nethraravindran
nethraravindran approved these changes Dec 17, 2018
View reviewed changes
Copy link
Contributor

@nethraravindran nethraravindran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nethraravindran nethraravindran mentioned this pull request Dec 17, 2018
Closed
@ianpartridge ianpartridge merged commit b47f582 into Kitura:master Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@nethraravindran nethraravindran nethraravindran approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nethraravindran nethraravindran

Labels

None yet

Projects

None yet

Milestone

2018.26

Development

Successfully merging this pull request may close these issues.

4 participants

@TheM4hd1 @CLAassistant @ianpartridge @nethraravindran