Bug - Fixed kolmafia failing to login/open

Status
Not open for further replies.
Veracity

Veracity

Developer
Staff member
I don't have time tonight. But, I understand the problem and I really want to log in to do one little thing before going to bed...
 
lostcalpolydude

lostcalpolydude

Developer
Staff member
It's only 1pm right now for me. I don't have much hope of figuring out how to fix it (my starting point was "how do I view HTTP headers?"), but I can certainly look over, test, and then commit a patch if someone else figures it out.
 
I

IronTetsubo

Member
Woot! Done! :) I can login now via kolmafia, via attached patch.

In all seriously, please do test a bit further before committing. I did login successfully and poked around a bit, but want to do a bit more testing w/ wireshark to ensure I'm really doing the right things.
 

Attachments

  • new_cookie.patch
    3.6 KB · Views: 73
I

IronTetsubo

Member
Hmm. further testing looks good for me, so far - indeed, wireshark says I'm sending the right 2 cookies (now) back to the server. :) Servers are nice and zippy, from a lack of all the kolmafia users right now...
 
C

Catch-22

Active member
Looks alright, definitely not the way I would've done it. Might be worth revising later with cleaner code (ie. no matcher) but otherwise I'd say if it works, it works.
 
Veracity

Veracity

Developer
Staff member
Revision 11172.

I am not happy about how this was handled - a server-side change dumped on us a few minutes after rollover with no advance warning whatsoever. It feels like a kick in the stomach. It was incredibly discourteous and exactly the way I have NOT come to expect CDM to treat me.

I better go to bed before I say something incredibly discourteous.
 
I

IronTetsubo

Member
Catch-22 said:
Looks alright, definitely not the way I would've done it. Might be worth revising later with cleaner code (ie. no matcher) but otherwise I'd say if it works, it works.
Click to expand...

Ah? out of curiosity, how come? I'm not a java developer at all, so happy to learn better ways. I figured the pattern matching was more "future proof", although to be fair, the format of a HTTP cookie is standard. I.e., if the kol devs do go and send the cookies on one line, then it's a relatively simple tweak to the pattern to match multiple groups and be done.
 
Q

Quase

New member
Hiya folks. My first post in the forum. I've been following the problem today, and I got the patch file you prepared, but I am not sure how to apply it. Could you point me in the right direction (my search efforts only returned instructions on how NOT to patch KolMafia during Crimbo :)). Thanks...
 
lostcalpolydude

lostcalpolydude

Developer
Staff member
Quase said:
Hiya folks. My first post in the forum. I've been following the problem today, and I got the patch file you prepared, but I am not sure how to apply it. Could you point me in the right direction (my search efforts only returned instructions on how NOT to patch KolMafia during Crimbo :)). Thanks...
Click to expand...

No need, http://builds.kolmafia.us/ will soon have 11172. If you want to know how to deal with patches for general use, you first need to follow the directions on the wiki to have a setup where a patch can be applied.
 
I

IronTetsubo

Member
Veracity just committed the changes - wait a bit for the hourly build of r11172 or later, and you'll be all set. Or you can download the source and build it from scratch, but if you're asking how to apply a patch, you're far better served waiting a bit for the auto build...
 
C

Catch-22

Active member
IronTetsubo said:
Ah? out of curiosity, how come? I'm not a java developer at all, so happy to learn better ways. I figured the pattern matching was more "future proof", although to be fair, the format of a HTTP cookie is standard. I.e., if the kol devs do go and send the cookies on one line, then it's a relatively simple tweak to the pattern to match multiple groups and be done.
Click to expand...

Take a look at Veracity's changes (she didn't use your patch). That's closer to what I would've done, but I would've future proofed it by handling all cookies (I don't know why we drop the path cookie anyway).
 
AlEcyler

AlEcyler

New member
Veracity said:
Revision 11172.

I am not happy about how this was handled - a server-side change dumped on us a few minutes after rollover with no advance warning whatsoever. It feels like a kick in the stomach. It was incredibly discourteous and exactly the way I have NOT come to expect CDM to treat me.

I better go to bed before I say something incredibly discourteous.
Click to expand...

I believe this was unintentional.
 
C

Catch-22

Active member
AlEcyler said:
I believe this was unintentional.
Click to expand...

I agree, CDM probably thought KoLmafia was handling cookies the same way a browser does, by sending them all back.

Given that KoLmafia hammers topmenu.php when the cookies aren't set right, I'd say they would've mentioned it if they knew :)
 
Bale

Bale

Minion
Thanks IronTetsubo. Today you get to be the hero.


Veracity said:
I am not happy about how this was handled - a server-side change dumped on us a few minutes after rollover with no advance warning whatsoever. It feels like a kick in the stomach. It was incredibly discourteous and exactly the way I have NOT come to expect CDM to treat me.
Click to expand...

On the main forum Pantsless says that CDM hadn't anticipated the damage to mafia. Maybe CDM really just assumed that mafia was robust enough to handle the cookie change.
 
I

IronTetsubo

Member
Bale said:
Thanks IronTetsubo. Today you get to be the hero.




On the main forum Pantsless says that CDM hadn't anticipated the damage to mafia. Maybe CDM really just assumed that mafia was robust enough to handle the cookie change.
Click to expand...

heh. you're welcome, but turns out I didn't - Catch-22 is correct, Veracity won with her commit powers. :) No big deal, glad it's working again.
 
Veracity

Veracity

Developer
Staff member
Catch-22 said:
I would've future proofed it by handling all cookies (I don't know why we drop the path cookie anyway).
Click to expand...
I don't know why, either.

We could remove the following lines:

Code: 
				if ( !cookie.startsWith( "appserver" ) && !cookie.startsWith( "PHPSESSID" ) )
				{
					continue;
				}
				int semi = cookie.indexOf( ";" );
				if ( semi != -1 )
				{
					cookie = cookie.substring( 0, semi );
				}
To make it use every cookie, including "path" which is glommed onto the PHPSESSID.
 
B

baethan

New member
I've been following this minisaga, and it was fascinating as a non-programmer to get a peek behind the scenes. And thanks so much for the quick fix!
 
Q

Quase

New member
baethan said:
I've been following this minisaga, and it was fascinating as a non-programmer to get a peek behind the scenes. And thanks so much for the quick fix!
Click to expand...

Same from me, although I am a programmer, but not in web environment. I also got the latest build and am already adventuring. Thanks again...
 
I

IronTetsubo

Member
Veracity said:
I don't know why, either.

We could remove the following lines:

Code: 
				if ( !cookie.startsWith( "appserver" ) && !cookie.startsWith( "PHPSESSID" ) )
				{
					continue;
				}
				int semi = cookie.indexOf( ";" );
				if ( semi != -1 )
				{
					cookie = cookie.substring( 0, semi );
				}
To make it use every cookie, including "path" which is glommed onto the PHPSESSID.
Click to expand...

er, what? http://en.wikipedia.org/wiki/HTTP_cookie#Domain_and_Path - "path" is not a cookie, it's an attribute of the cookie setting the context path for which the cookie is valid. you can basically ignore it for these purposes, but don't set/pass back a "path" cookie. Otherwise, sure, go ahead and pass back each cookie.
 
Status
Not open for further replies.
Top