[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