Discussion: should we revert 0-argument visit_url to not follow redirects?

heeheehee

Developer
Staff member
Thanks for the fix.

Something else I noticed while looking into this just now is that Mafia silently follows redirects without telling the browser, so the browser thinks that a redirect never happened. This is mostly unrelated to a bug wherein the wrong relay script fires after Mafia follows redirects (the problem there is that visit_url() looks at RelayRequest.redirectLocation, but the script called is based on RelayRequest.getBasePath()).
 

Veracity

Developer
Staff member
Does it? My experience is that it passes them down to the browser and lets the browser issue a new request.

GenericRequest.handleServerRedirect returns true - i.e., stop - if ( this instanceof RelayRequest ).

What code path are you looking at?
 

lostcalpolydude

Developer
Staff member
Not based on looking at code, but when a choice redirects to a fight my fight.ash script (which just shows the old combat form) doesn't run the first round.
 

heeheehee

Developer
Staff member
I'm just comparing the HTTP headers in-browser with the actual requested URLs. In particular:

Browser:
Code:
POST /choice.php HTTP/1.1
Host: 127.0.0.1:60081
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
DNT: 1
Referer: http://127.0.0.1:60081/choice.php
Cookie: charpwd=200; chatpwd=305; inventory=295936
Connection: keep-alive

HTTP/1.1 200 OK
Date: Mon Feb 22 16:35:32 EST 2016
Server: KoLmafia v17.2
Content-Type: text/html; charset=UTF-8
Cache-Control: no-cache, must-revalidate
Pragma: no-cache
Connection: close
(note the 200 OK; also, this showed up as a request for choice.php and had the response body of fight.php)

Debug log of that same request:
Code:
Requesting: http://www.kingdomofloathing.com/choice.php?pwd&whichchoice=46&option=3
3 request properties
Field: Cookie = [appserver=www9; PHPSESSID=*snipped*]
Field: User-Agent = [KoLmafia v17.2]
Field: Content-Type = [application/x-www-form-urlencoded]

Retrieving server reply...

Retrieved: http://www.kingdomofloathing.com/choice.php?pwd&whichchoice=46&option=3
11 header fields
Field: null = [HTTP/1.1 302 Found]
Field: Cache-Control = [no-store, no-cache, must-revalidate, post-check=0, pre-check=0]
Field: Server = [nginx/1.0.15]
Field: Connection = [keep-alive]
Field: Pragma = [no-cache]
Field: Expires = [Thu, 19 Nov 1981 08:52:00 GMT]
Field: Content-Length = [0]
Field: Date = [Mon, 22 Feb 2016 21:36:13 GMT]
Field: Location = [fight.php?ireallymeanit=1456176973]
Field: X-Powered-By = [PHP/5.3.3]
Field: Content-Type = [text/html; charset=UTF-8]

class net.sourceforge.kolmafia.request.RelayRequest
Connecting to fight.php...

Requesting: http://www.kingdomofloathing.com/fight.php?ireallymeanit=1456176973
2 request properties
Field: Cookie = [appserver=www9; PHPSESSID=*snipped*]
Field: User-Agent = [KoLmafia v17.2]

Retrieving server reply...

Retrieved: http://www.kingdomofloathing.com/fight.php?ireallymeanit=1456176973
10 header fields
Field: Transfer-Encoding = [chunked]
Field: null = [HTTP/1.1 200 OK]
Field: Cache-Control = [no-store, no-cache, must-revalidate, post-check=0, pre-check=0]
Field: Server = [nginx/1.0.15]
Field: Connection = [keep-alive]
Field: Pragma = [no-cache]
Field: Expires = [Thu, 19 Nov 1981 08:52:00 GMT]
Field: Date = [Mon, 22 Feb 2016 21:36:13 GMT]
Field: X-Powered-By = [PHP/5.3.3]
Field: Content-Type = [text/html; charset=UTF-8]

(using the spooky forest noncombat to enter a fight with a spooky vampire)
 

heeheehee

Developer
Staff member
Code:
-----From Browser-----
POST /choice.php HTTP/1.1
Host: 127.0.0.1:60081
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
DNT: 1
Referer: http://127.0.0.1:60081/choice.php
Cookie: charpwd=200; chatpwd=305; inventory=295936
Connection: keep-alive
Content-Type: application/x-www-form-urlencoded
Content-Length: 60
pwd=f2f9526e06eccaec2c84c5da494aae34&whichchoice=46&option=3
----------
Starting relay script: choice.ash
class net.sourceforge.kolmafia.request.RelayRequest
Connecting to choice.php...

Requesting: http://www.kingdomofloathing.com/choice.php?pwd&whichchoice=46&option=3
3 request properties
Field: Cookie = [appserver=www9; PHPSESSID=*snipped*]
Field: User-Agent = [KoLmafia v17.2]
Field: Content-Type = [application/x-www-form-urlencoded]

Retrieving server reply...

Retrieved: http://www.kingdomofloathing.com/choice.php?pwd&whichchoice=46&option=3
11 header fields
Field: null = [HTTP/1.1 302 Found]
Field: Cache-Control = [no-store, no-cache, must-revalidate, post-check=0, pre-check=0]
Field: Server = [nginx/1.0.15]
Field: Connection = [keep-alive]
Field: Pragma = [no-cache]
Field: Expires = [Thu, 19 Nov 1981 08:52:00 GMT]
Field: Content-Length = [0]
Field: Date = [Mon, 22 Feb 2016 22:37:43 GMT]
Field: Location = [fight.php?ireallymeanit=1456180663]
Field: X-Powered-By = [PHP/5.3.3]
Field: Content-Type = [text/html; charset=UTF-8]

class net.sourceforge.kolmafia.request.RelayRequest
Connecting to fight.php...

Requesting: http://www.kingdomofloathing.com/fight.php?ireallymeanit=1456180663
2 request properties
Field: Cookie = [appserver=www9; PHPSESSID=*snipped*]
Field: User-Agent = [KoLmafia v17.2]

Retrieving server reply...

Retrieved: http://www.kingdomofloathing.com/fight.php?ireallymeanit=1456180663
10 header fields
Field: Transfer-Encoding = [chunked]
Field: null = [HTTP/1.1 200 OK]
Field: Cache-Control = [no-store, no-cache, must-revalidate, post-check=0, pre-check=0]
Field: Server = [nginx/1.0.15]
Field: Connection = [keep-alive]
Field: Pragma = [no-cache]
Field: Expires = [Thu, 19 Nov 1981 08:52:00 GMT]
Field: Date = [Mon, 22 Feb 2016 22:37:43 GMT]
Field: X-Powered-By = [PHP/5.3.3]
Field: Content-Type = [text/html; charset=UTF-8]

Retrieving server reply
ResponseText has 35202 characters.
<html>*snipped*</html>
Encounter: spooky vampire
Round 0: oneofmanynames wins initiative!
Round 0: oneofmanynames casts CURSE OF WEAKSAUCE!
Round 1: spooky vampire drops 1 attack power.
Round 1: spooky vampire drops 2 defense.
Processing result:  MP: -8
Round 1: oneofmanynames casts SAUCEGEYSER!
Round 2: spooky vampire takes 314 damage.
Round 2: spooky vampire drops 4 attack power.
Round 2: spooky vampire drops 2 defense.
Round 2: oneofmanynames wins the fight!
Parsing result: You gain 50 Mana Points
After Battle: You gain 50 Mana Points
Processing result:  MP: 50
Parsing result: You gain 11 Meat
You gain 11 Meat
Processing result:  Meat Gained: 11
After Battle: Fez bends its brim into an approximation of a smile. (+1 Stat)
Parsing result: You gain 3 Muscleboundness
After Battle: You gain 3 Muscleboundness
Processing result:  Substats: 3 / 0 / 0
Parsing result: You gain 3 Magicalness
After Battle: You gain 3 Magicalness
Processing result:  Substats: 0 / 3 / 0
Parsing result: You gain 2 Cheek
After Battle: You gain 2 Cheek
Processing result:  Substats: 0 / 0 / 2
Processing result:  MP: -24
Finished relay script: choice.ash
-----To Browser-----
HTTP/1.1 200 OK
Date: Mon Feb 22 17:37:01 EST 2016
Server: KoLmafia v17.2
Content-Type: text/html; charset=UTF-8
Cache-Control: no-cache, must-revalidate
Pragma: no-cache
Connection: close
----------
Is this what you wanted?
 

heeheehee

Developer
Staff member
Renamed choice.ash, and it redirected just fine in-browser.

I'm not sure if there's a correct resolution, since the "bug" I mentioned seems to be tied to the 0-parameter version of visit_url() following redirects, instead of triggering a redirect.
 

Veracity

Developer
Staff member
Yes indeed. I made relay scripts follow redirects a while ago as a result of a bug report or feature request.

The alternative would be for the relay script to be passed a blank buffer and some indication that it was a redirect, and to simply return and let KoLmafia pass the redirect down to the browser. The browser would then follow the redirect and submit a new request, which would (potentially) make a different relay script run.

That seems like the better - read "more correct" - solution.

Go for it. :)
 

heeheehee

Developer
Staff member
From that thread:
OK, I'm beginning to be convinced that visit_url() - the one with no argument, i.e., the one that a relay script uses to get the "current" page - should follow redirects.

If you look at the Right Side of the Tracks, you see Spookyraven Manor with a link to manor.php.
When you click on it, manor.php redirects to place.php?whichplace=spookyraven1

So, Bale's "manor.php" is called when you click on manor.php, it calls visit_url() and gets a blank page, since it redirects to place.php?whichplace=spookyraven1, and gets a "Index 7 out of bounds (manor.ash, line 66)" when it tries to insert its markup in the blank page.

So, is the solution to have visit_url() follow the redirect automatically - and have the script continue to be manor.ash - or make Bale rename his script to place.ash and handle only whichplace=spookyraven1, thereby conflicting with other scripts that might want to handle other "places"?

I think the former.
These days, the latter would probably be better, as one could instead create place.spookyraven1.ash. On the other hand, I'm hesitant to revert functionality introduced several years earlier.
 

Veracity

Developer
Staff member
Well lets move this series of posts over to a new discussion (so we can close this Bug Report) and see what the greater Scripting Community thinks.

I also have come around to thinking that an xxx.ash relay script should ONLY handle xxx.php.

If they get a redirect and we pass back an empty buffer, they should simply return an empty buffer. We could make current_redirect_location() return a string, in case it wants to see what was going on, but I wonder if there is actually a legitimate use for pre-empting a different relay script as well "fooling" the briowser with the response from a different URL than it submitted.
 

Veracity

Developer
Staff member
Note that visit_url called NOT from within a RelayRequest (i.e., from the browser) will use a GenericRequest, which will automatically follow redirects for you.
 

Bale

Minion
For what it's worth there's some context being lost in the above quote about my manor.php override due to the passage of time. Back then KoLmafia didn't support a place.spookyraven1.ash override. It was place.ash or nothin! Thank goodness for that improvement. Once KoLmafia would allow me to make separate overrides for each whichplace value, I switched over to the new URLs.

Back to discussion of redirection, I feel that if mafia redirects a url for whatever reason, we are tampering with the game by refusing to follow the URL. There's usually a reason for the redirect after all.
 
I have an 'awesomemenu.ash' relay script which modfies my iconic topmenu.

When you unlock new locations, KoL asks the topmenu frame to show 'topmenu.php' which is then redirected to 'awesomemenu.php' if you have iconic topmenu enabled.

Currently those refreshes only work if I do not have a 'topmenu.ash' relay script, otherwise I need to manually refresh the topmenu frame to get the 'awesomemenu.ash' relay script working.

I'm not sure I completely understand the change Veracity is proposing. Either:
A) The redirected refreshes would work regardless of if I have a 'topmenu.ash' script. (As 'topmenu.ash' would not be called for a request redirected to 'awesomemenu.php' )
B) The redirected refreshes would not work at all. (As 'awesomemenu.ash' would not be called for a request to 'topmenu.php' regardless of whether it is redirected to 'awesomemenu.php' )

Which of those is the correct reading?
 

heeheehee

Developer
Staff member
The result of reverting this revision would be A) --- a topmenu.ash override would probably end up getting a blank page to override (which wouldn't be displayed anyways, since the browser would follow the redirect to awesomemenu.php, where that override would trigger).
 

Veracity

Developer
Staff member
1) The browser requests topmenu.php
2) topmenu.ash is called. It asks KoL for topmenu.php
3) KoL redirects to awesomenu.php
4) KoLmafia gives the relay script a blank page.
5) The relay script returns a blank page to KoLmafia
6) KoLmafia passes the redirect down to the browser
7) The browser requests awesomemenu.php
8) awesomemenu.ash is called. It asks KoL for awesomemenu.php
9) KoL returns the page for awesomemenu
10) the relay script modifies it as desired and returns the page to KoLmafia
11) KoLmafia sends the modified page down to the browser
12) The browser displays the page, complete with your script's customizations.

That is how it would work if we do NOT follow redirections in the relay script's visit_url() call. Which is to say, what we are proposing/discussing here.

I guess a question would be: what do we do if a relay script (topmenu.php) asks for the page and gets a blank page + redirect, but then returns a non-blank page to KoLmafia?

1) Ignore it, and simply pass the redirect down to the browser
2) Assume the script is overriding the page regardless of the redirect, and pass the customized page down to the browser
 

xKiv

Active member
If they get a redirect and we pass back an empty buffer, they should simply return an empty buffer.

Is this how the script will know it got a redirect?

(also, what if the script wants to get additional data from a different url, and *that* redirects?)


1) Ignore it, and simply pass the redirect down to the browser
2) Assume the script is overriding the page regardless of the redirect, and pass the customized page down to the browser

I believe it has to be up to the script to know what it was doing, and therefore what a redirect means. So that's option #2.
 
Could scripts be forced to opt-in in order to handle redirects? That should prevent naive relay scripts having unintended behavior for redirected requests, while still allowing flexibility for scripts that know what they're doing.

For example, you could maybe use different versions of visit_url() and write() - either new functions or overload the existing ones.

EDIT - I guess Veracity's suggestion of current_redirect_location() could handle that case for visit_url(), in the event it receives a blank page.
 
Last edited:

xKiv

Active member
I guess calling a redirect-aware function is a good way to tell kolmafia that the script is redirect aware.

Now, should forced relayed redirects happen even when the visit_url call was made to a different base than the relay? (i.e. topmenu.ash calls visit_url('somethingother.php') which redirects)
 

Veracity

Developer
Staff member
Well, we are only talking about the 0-argument form of visit_url, which is what a relay script uses to actually fetch the page that the browser asked for.

If you specify a path, you are expected to know if and where it will redirect to and behave appropriately. You are proposing that we automatically do that for scripts? So, if "using" an item redirects to a fight, that we follow that redirect and get the first page of the fight? I wonder what that would break...
 
Top