[PATCH 3/4] http: Fix redirect handling in auth form loop

David Woodhouse dwmw2 at infradead.org
Sun Feb 17 17:44:43 EST 2013


On Sun, 2013-02-17 at 14:28 -0800, Kevin Cernekee wrote:
> On Sun, Feb 17, 2013 at 2:03 PM, Woodhouse, David
> <david.woodhouse at intel.com> wrote:
> > On Sat, 2013-02-16 at 16:18 -0800, Kevin Cernekee wrote:
> >> The refactored openconnect_obtain_cookie() loop tried to post the
> >> challenge/response data to index.html, preventing successful login.
> >> This patch changes the logic so that it will honor the new "action"
> >> attribute if present.
> >
> > Point of order: the 'action' attribute isn't new; it's been there for
> > years. Since commit 7ba752f8 in April 2009, to be precise, when the
> > struct auth_form was first introduced.
> 
> Clarification: by "new" I meant that "action" did not contain the same
> value as what had showed up in the previous form.  e.g. the old value
> is "index.html" but the new value is "challenge.html"

Ah right, that makes more sense then :)

> Fabian is rerunning his tests against 91462d2e + my 4 commits now.
> 
> I prototyped the safe_realloc() change, if you're interested.

Hm. I'm in two minds about this. We have half as many calls to
gnutls_realloc() as we do to plain realloc()... do we wrap those too?

Why doesn't the compiler *complain* about the 'foo = realloc(foo, …)'
idiom anyway? Surely it should?

I'm very tempted to suggest that we should just be making people learn
the f***ing C language and NOT DO THAT. Yes, I know I'm the main
offender; I'd be a whole lot more circumspect if I wasn't. :)

> > We now have two callers of parse_xml_response() which do, and two which
> > don't. I'm suspect that's a bad thing, and we should put it *into*
> > parse_xml_response() rather than leaving it to the caller...
> 
> For the XML POST case, I would expect no "action" attribute;
> everything always seems to get posted to the root URL.
> 
> For the CSD case, the server probably shouldn't be changing the post
> URL just because we refreshed the login form post-CSD.  If it does
> anyway, we might as well follow it.
> 
> So this sounds like it would work OK.

OK, I'll put it together and test the non-xmlpost side it. Thanks.

-- 
dwmw2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6171 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20130217/0ae2561d/attachment.bin>


More information about the openconnect-devel mailing list