redirect error ! isRedirect not working #69
Comments
|
@mostafadahab thanks for pointing out the error. I'm not understanding what you mean by "isRedirect" not working. Goreq isRedirect method is tested here: https://github.com/franela/goreq/blob/master/goreq_test.go#L605 and it seems to be doing what it's supposed to do. Can you please provide an example where it's not doing what it should do?. One thing I see is that Goreq MaxRedirects is initialized with a zero value whereas the link that you sent says that Golang default http client supports up to 10 redirects. Is that what you're refering? Thanks. |
|
thank you very much for quick response ! |
|
@mostafadahab can you please paste some code here and the compelte URL you're trying to access so we can give it a shot?. Thanks. |
package main
import (
"fmt"
"./goreq"
)
func main() {
/*for {
res, err := goreq.Request{ Uri: "http://www.google.com" ,Timeout:60 * time.Second}.Do()
if err != nil {
continue
}
fmt.Print(res.Body.ToString())
break
}*/
amflogin("","")
/*req:
res, err := goreq.Request{ Uri: "http://www.google.com" ,Timeout:60 * time.Second}.Do()
if err != nil {
goto req
}
fmt.Print(res.StatusCode)*/
}
func amflogin(email,password string) (points string , err error){
req := goreq.Request{Method: "POST",MaxRedirects: -1,Proxy:"http://127.0.0.1:8888",Uri: "http://addmefast.com",ContentType:"application/x-www-form-urlencoded",Body: "email=smartspammer@gmail.com&password=abc123abc123&login_button=Login",Accept:"text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8",UserAgent:"Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.71 Safari/537.36"}
res,err := req.Do()
if err != nil {
fmt.Println(err.Error())
}
fmt.Println(res.Body.ToString())
return "test",nil
} |
|
@mostafadahab I'm looking into is as I've noticed that something is strange with the site that you sent me. If you run the request against www.google.com (per your example) or some other site that returns 302 then by default Goreq won't follow redirects unless MaxRedirects is specified. However, for the POST that you're sending I'm seeing that Golang is following the redirect either way. I'm looking into it and I'll answer back shortly |
|
i think it's about this code in client.go file : CheckRedirect specifies the policy for handling redirects.
42 // If CheckRedirect is not nil, the client calls it before
43 // following an HTTP redirect. The arguments req and via are
44 // the upcoming request and the requests made already, oldest
45 // first. If CheckRedirect returns an error, the Client's Get
46 // method returns both the previous Response and
47 // CheckRedirect's error (wrapped in a url.Error) instead of
48 // issuing the Request req.
49 //
50 // If CheckRedirect is nil, the Client uses its default policy,
51 // which is to stop after 10 consecutive requests.
52 CheckRedirect func(req *Request, via []*Request) error |
|
@mostafadahab I've found the error and fixed it here: fa5798b. Amazingly seems like a bug in Golang http client that by default it doesn't follow redirects when Method is not explicitly set in http://golang.org/src/pkg/net/http/client.go?s=5094:5155#L138 checks for Method explicitly and performs redirects based on that. As Goreq uses the Expect this PR to be merged today once a collaborator reviews it. |
|
Great Job !! 👍 |
|
@mostafadahab that has been fixed in this commit also: c7a1e8a can you please checkout the redirect_fix branch and give it a shot with your test scenario? |
|
i have tested the new code same problem ! |
|
@mostafadahab this is the test i'm running package main
import (
"fmt"
"github.com/franela/goreq"
)
func main() {
req := goreq.Request{
Method: "POST",
Uri: "http://addmefast.com",
ContentType: "application/x-www-form-urlencoded",
Body: "email=smartspammer@gmail.com&password=abc123abc123&login_button=Login",
Accept: "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8",
UserAgent: "Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.71 Safari/537.36",
}
res, _ := req.Do()
body, _ := res.Body.ToString()
fmt.Println(body, res.StatusCode, res.Header)
}and this is the output i'm getting: As you can see redirect is not being handled (because MaxRedirects is not specified) and I'm getting the corresponding request headers. What it's not working for you? |
|
Very strange !! i discover the problem was in proxy !! when remove proxy no redirects follow but when adding proxy the redirect working !! |
|
i have fix the error by rise this code if r.Proxy != "" {
proxyUrl, err := url.Parse(r.Proxy)
if err != nil {
// proxy address is in a wrong format
return nil, &Error{Err: err}
}
if proxyTransport == nil {
proxyTransport = &http.Transport{Dial: defaultDialer.Dial, Proxy: http.ProxyURL(proxyUrl)}
proxyClient = &http.Client{Transport: proxyTransport}
} else {
proxyTransport.Proxy = http.ProxyURL(proxyUrl)
}
transport = proxyTransport
client = proxyClient
}before this code client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
if len(via) > r.MaxRedirects {
redirectFailed = true
return errors.New("Error redirecting. MaxRedirects reached")
}
return nil
}to become the full Do func : func (r Request) Do() (*Response, error) {
var req *http.Request
var er error
var transport = defaultTransport
var client = defaultClient
var redirectFailed bool
r.Method = valueOrDefault(r.Method, "GET")
if r.Proxy != "" {
proxyUrl, err := url.Parse(r.Proxy)
if err != nil {
// proxy address is in a wrong format
return nil, &Error{Err: err}
}
if proxyTransport == nil {
proxyTransport = &http.Transport{Dial: defaultDialer.Dial, Proxy: http.ProxyURL(proxyUrl)}
proxyClient = &http.Client{Transport: proxyTransport}
} else {
proxyTransport.Proxy = http.ProxyURL(proxyUrl)
}
transport = proxyTransport
client = proxyClient
}
client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
if len(via) > r.MaxRedirects {
redirectFailed = true
return errors.New("Error redirecting. MaxRedirects reached")
}
return nil
}
if r.Insecure {
transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
} else if transport.TLSClientConfig != nil {
// the default TLS client (when transport.TLSClientConfig==nil) is
// already set to verify, so do nothing in that case
transport.TLSClientConfig.InsecureSkipVerify = false
}
b, e := prepareRequestBody(r.Body)
if e != nil {
// there was a problem marshaling the body
return nil, &Error{Err: e}
}
if r.QueryString != nil {
param, e := paramParse(r.QueryString)
if e != nil {
return nil, &Error{Err: e}
}
r.Uri = r.Uri + "?" + param
}
var bodyReader io.Reader
if b != nil && r.Compression != nil {
buffer := bytes.NewBuffer([]byte{})
readBuffer := bufio.NewReader(b)
writer, err := r.Compression.writer(buffer)
if err != nil {
return nil, &Error{Err: err}
}
_, e = readBuffer.WriteTo(writer)
writer.Close()
if e != nil {
return nil, &Error{Err: e}
}
bodyReader = buffer
} else {
bodyReader = b
}
req, er = http.NewRequest(r.Method, r.Uri, bodyReader)
if er != nil {
// we couldn't parse the URL.
return nil, &Error{Err: er}
}
// add headers to the request
req.Host = r.Host
req.Header.Add("User-Agent", r.UserAgent)
req.Header.Add("Content-Type", r.ContentType)
req.Header.Add("Accept", r.Accept)
if r.Compression != nil {
req.Header.Add("Content-Encoding", r.Compression.ContentEncoding)
req.Header.Add("Accept-Encoding", r.Compression.ContentEncoding)
}
if r.headers != nil {
for _, header := range r.headers {
req.Header.Add(header.name, header.value)
}
}
//use basic auth if required
if r.BasicAuthUsername != "" {
req.SetBasicAuth(r.BasicAuthUsername, r.BasicAuthPassword)
}
timeout := false
var timer *time.Timer
if r.Timeout > 0 {
timer = time.AfterFunc(r.Timeout, func() {
transport.CancelRequest(req)
timeout = true
})
}
res, err := client.Do(req)
if timer != nil {
timer.Stop()
}
if err != nil {
if !timeout {
switch err := err.(type) {
case *net.OpError:
timeout = err.Timeout()
case *url.Error:
if op, ok := err.Err.(*net.OpError); ok {
timeout = op.Timeout()
}
}
}
var response *Response
//If redirect fails we still want to return response data
if redirectFailed {
response = &Response{StatusCode: res.StatusCode, ContentLength: res.ContentLength, Header: res.Header, Body: &Body{reader: res.Body}}
}
return response, &Error{timeout: timeout, Err: err}
}
if r.Compression != nil && strings.Contains(res.Header.Get("Content-Encoding"), r.Compression.ContentEncoding) {
compressedReader, err := r.Compression.reader(res.Body)
if err != nil {
return nil, &Error{Err: err}
}
return &Response{StatusCode: res.StatusCode, ContentLength: res.ContentLength, Header: res.Header, Body: &Body{reader: res.Body, compressedReader: compressedReader}}, nil
} else {
return &Response{StatusCode: res.StatusCode, ContentLength: res.ContentLength, Header: res.Header, Body: &Body{reader: res.Body}}, nil
}
} |
|
@mostafadahab I believe I know why it failed when setting a proxy. Even though raising the code worked for you I believe it's not the best solution. Let me code a test to reproduce the scenario and I'll fix the code in a couple of mins. |
|
Do it man nice work ;) 👍 ,, but the issue "removing headers like UserAgent etc.." still there ! |
|
@mostafadahab I've been looking for the headers issue and yes, it seems like it's something from the go client. There's an issue reported here: https://code.google.com/p/go/issues/detail?id=4800&q=redirect&colspec=ID%20Status%20Stars%20Release%20Owner%20Repo%20Summary. However, I believe that we can do something on our end to implement it. Let me investigate a bit more and I'll get back to you shortly |
|
You seem like be doing a great work !! 2 small things
thanks in advance |
|
@mostafadahab the proxy redirect issue is already fixed. I'll be adding the cookie and header redirect support later today. Regarding to contributions, you're already doing a great job by using our library and spotting bugs which will make it better for some other people. We're doing this just to return something to the open source community. |
|
@mostafadahab header forward when redirect has been added here: 3478220 Yo need to set the RedirectHedears flag to |
|
@mostafadahab did this finally work for you? |
|
@marcosnils thanks for update i was waiting for adding cookies support 🔨 |
|
any updates ?? |
|
@mostafadahab sorry, I've started the work but I got occupied with some other stuff and I couldn't complete it. This weekend I hope to dedicate it enough time to have it ready. Contributions are very welcome though. You can make the modifications yourself and propose the changes so we can move faster. |
|
@mostafadahab just wanted to let u know that cookie support has been included in #78 thanks to @bfontaine . |
|
good job ! i tried the new version of goreq but still there issue in redirect with headers and i can't find RedirectHedears ! and the cookie jar not saving set-cookie and resend it in redirect requests |
|
@mostafadahab can have you updated the library locally?. The RedirectHeaders flag can be set here: |
|
@mostafadahab regarding cookies on redirection it hasn't been done because of security concerns. It's not safe to automatically redirect cookies as the final URL might get sensitive cookies information. If you still want do send the cookies anyways, you can redefine the CheckRedirect function here: https://github.com/franela/goreq/blob/master/goreq.go#L276 |
|
@marcosnils RedirectHeaders works the problem was not having the last version i say the cookie jar must attached for example if i'am make login to facebook.com and facebook redirect to facebook.com/home.php the must have cookies ! if not will log me out ! |
|
@mostafadahab makes sense. We can add a RedirectCookies flag into goreq.Request for that purpose. |
|
thanks alot you're awesome i'm developing me skills in go but it's still weak when i'm ready i will try to contribute |
|
@mostafadahab speaking with @xetorthio I've realized that cookies are currently being forwarded within redirects. Cookies it's just another header in the request, so if you do the following request: req := goreq.Request{
Method: "GET",
Uri: "http://somedomain.com/redirects",
MaxRedirects: 2,
RedirectHeaders: true,
}
req.AddCookie(&http.Cookie{Name: "Test", Value: "Value"})
res, _ := req.Do()Then the Per your facebook example seems like you want What I would recommend you to do is to make the login in two steps
|
|
or just assign global cookiejar to each request @marcosnils thanks man 👍 😍 |
isRedirect not working maybe it's default settings for go client see: http://golang.org/src/pkg/net/http/client.go
and what about cookies !!
thanks for cool lib 👍
The text was updated successfully, but these errors were encountered: