Bug - Fixed Store Manager "save prices" fails with lots of items in store
- Thread starter ungawa
- Start date
I'm not seeing a difference between the request we make from Store Manager and the request sent from the relay browser. I think we also need to get a debug log of a relay request to see what KoL does differently, but that's only likely to be useful in the case where the Store Manager request fails.
I think I have generated such a debug log. I ran OCD inventory control to get all that min price crap back in my store and confirmed it gave the same "stringindexoutofbounds" exception as before.
I started the debug log, pressed the Mafia store manager's "save prices" button, which failed. Then in the relay browser I went to manageprices.php and pressed "Update Prices", which succeeded. In both cases, I changed the price of one item (Helvella Haemophilia mushroom) just to be consistent.
It's a bit beyond me to compare the results, but I do see long url's for both requests.
Edit: I see in the first debug log I uploaded, the mafia "save prices" request was sorted by descending prices, so I recreated the test with it sorted alphabetically so the mafia "save prices" request would more closely resemble the relay browser "update prices" request.
The mafia "save prices" url has 50,131 characters. The relay browser "update prices" url has 50,119 characters. I also see a "pwd&" near the start of the relay browser request but not the store manager request. Some items are alphabetized in a different order between the requests, such as "a little sump'm sump'm" where mafia ignores the space for ordering.
Edit: of course, on closer inspection I see that log has my pwd in some of the urls. All I need to do is relog to prevent anyone from being able to abuse that info right?
Last edited:
Yes.
xKiv
Active member
I don't. The request from browser seems to be a POST, and the long data is in request body, which does not have the same limit as URL.
"Requesting: https://www.kingdomofloathing.com/manageprices.php?action=update&..." appears in both the RelayRequest and the ManageStoreRequest. There's no way to tell whether GET or POST was used in the ManageStoreRequest, although just tracing through the code, I see no reason why it wouldn't have used POST.
xKiv
Active member
I am looking at DEBUG...Alphabetical, where I see this sequence from browser:
GET submitnewchat.php ... /goto manageprices.php
GET manageprices.php (to display the page)
bunch of javascript, css, newchatmessages
POST manageprices.php (with action=updateprices&pwd=... in the body)
I conclude that vanilla kol uses POST requests to update prices.
(I also see that the first manageprices.php request was a GET - because 1) it's so written in the debug log 2) it has all that data in URL - and it's the one made by mafia because 1) it's not preceded by a request "From browser" and 2) ungawa reported the order of perations as "update in mafia THEN update in vanilla")
The first manageprices.php request was a GET because that's fetching the page in browser. As I already said, there's no logging of HTTP status codes for internal Mafia requests.
These are the three requests to manageprices.php: first, Mafia
Then, fetching manageprices.php via a GET request:
And finally, actually submitting the form:
As I've said already, there is no evidence that Mafia is using a GET request to update prices. The same observations can be made from either debug log.
These are the three requests to manageprices.php: first, Mafia
Then, fetching manageprices.php via a GET request:
And finally, actually submitting the form:
As I've said already, there is no evidence that Mafia is using a GET request to update prices. The same observations can be made from either debug log.
xKiv
Active member
Does mafia not log request bodies from internal requests?
And wouldn't a POST have Content-Length header (or does mafia not log that for internal requests)?\
ETA: although, looking at the code, it *should* be a POST.
ETA2: it's hitting the "this.formConnection.setRequestMethod( "POST" );" line, at least.
And wouldn't a POST have Content-Length header (or does mafia not log that for internal requests)?\
ETA: although, looking at the code, it *should* be a POST.
ETA2: it's hitting the "this.formConnection.setRequestMethod( "POST" );" line, at least.
Last edited:
xKiv
Active member
OK, so I stopped looking at request method, and instead started comparing the request data.
The only thing I can see is that mafia sends the items in a slightly different order.
... aaand ... the grouping is different? Mafia has 100 things in price0, vanilla has 101 things in price0?
(but the item IDs, prices, and limits are the same, just in different groups)
The only thing I can see is that mafia sends the items in a slightly different order.
... aaand ... the grouping is different? Mafia has 100 things in price0, vanilla has 101 things in price0?
(but the item IDs, prices, and limits are the same, just in different groups)
xKiv
Active member
Also possibly of note: when I go manage prices in vanilla kol, backoffice.php is used (not manageprices.php), and all items are in price[], limits[]. There is no price0[], price1[], price2[], ...
(that's with ~ 118 items in store)
Maybe manageprices.php is now a "backwards compatibility layer", and doesn't handle everything perfectly well?
(that's with ~ 118 items in store)
Maybe manageprices.php is now a "backwards compatibility layer", and doesn't handle everything perfectly well?
Code:
public ManageStoreRequest( final int[] itemId, final int[] prices, final int[] limits )
{
super( "manageprices.php" );
this.addFormField( "action", "update" );
int formInt;
this.requestType = ManageStoreRequest.PRICE_MANAGEMENT;
for ( int i = 0; i < itemId.length; ++i )
{
formInt = ( ( i - 1 ) / 100 ); //Group the form fields for every 100 items.
this.addFormField( "price" + formInt + "[" + itemId[ i ] + "]", prices[ i ] == 0 ? "" : String.valueOf( Math.max(
prices[ i ], Math.max( ItemDatabase.getPriceById( itemId[ i ] ), 100 ) ) ) );
this.addFormField( "limit" + formInt + "[" + itemId[ i ] + "]", String.valueOf( limits[ i ] ) );
}
}StoreManageFrame.java:
Code:
@Override
public void actionConfirmed()
{
if ( !InputFieldUtilities.finalizeTable( StoreManageFrame.this.manageTable ) )
{
return;
}
KoLmafia.updateDisplay( "Compiling reprice data..." );
int rowCount = StoreManageFrame.this.manageTable.getRowCount();
int[] itemId = new int[ rowCount ];
int[] prices = new int[ rowCount ];
int[] limits = new int[ rowCount ];
SoldItem[] sold = new SoldItem[ StoreManager.getSoldItemList().size() ];
StoreManager.getSoldItemList().toArray( sold );
for ( int i = 0; i < rowCount; ++i )
{
String item = (String) StoreManageFrame.this.manageTable.getValueAt( i, 0 );
itemId[ i ] = ItemDatabase.getItemId( item );
prices[ i ] = ( (Integer) StoreManageFrame.this.manageTable.getValueAt( i, 1 ) ).intValue();
int cheapest = ( (Integer) StoreManageFrame.this.manageTable.getValueAt( i, 2 ) ).intValue();
if ( cheapest >= 1000000 && prices[ i ] < cheapest * 0.15 )
{
String message = item + ": the price is less than 15% of the cheapest in the mall, continue?";
if ( !InputFieldUtilities.confirm( message ) )
{
return;
}
}
int ilim = (Integer) StoreManageFrame.this.manageTable.getValueAt( i, 4 );
limits[ i ] = ilim > 0 ? ilim : 0;
}
RequestThread.postRequest( new ManageStoreRequest( itemId, prices, limits ) );
}Looks like KoL submits a GET request for
So, almost exactly the same interface.
The _ field seems to be an optional UNIX timestamp.
Multiple requests can be bundled together and even sent off via POST via
Looks like prices always need to be specified. If a limits isn't specified, it gets set to 0.
The responseText looks something like this:
Note the comment with the JSON object mapping description ID to price & limit.
Code:
backoffice.php?action=updateinv&price[1907]=132&limit[1907]=0&ajax=1&pwd&_=1460864632340The _ field seems to be an optional UNIX timestamp.
Multiple requests can be bundled together and even sent off via POST via
Code:
backoffice.php?action=updateinv&ajax=1&pwd&price[1907]=131&limit[1907]=1&price[5254]=7800&limit[5254]=0Looks like prices always need to be specified. If a limits isn't specified, it gets set to 0.
The responseText looks something like this:
HTML:
<script type="text/javascript">if (window.updateInv) updateInv([])</script><script type="text/javascript">if (!window.updateInv && parent.mainpane.updateInv) parent.mainpane.updateInv([])</script><center><table width=95% cellspacing=0 cellpadding=0><tr><td style="color: white;" align=center bgcolor=blue><b>Results:</b></td></tr><tr><td style="padding: 5px; border: 1px solid blue;"><center><table><tr><td>8-ball updated (price: 130, limit: 0)<!-- U:{"631597539":{"price":130,"lim":0}} --></td></tr></table></center></td></tr><tr><td height=4></td></tr></table></center>Note the comment with the JSON object mapping description ID to price & limit.
xKiv
Active member
For reference, this is the javascript that handles "update all" (only the part for "save changes"):
(and i.attr('name') will be price[$itemid] for price, limit[$itemid] for limits)
Code:
var i;
var params = {
ajax: 1,
action: 'updateinv',
pwd: pwd
};
$('.deets').each(function (e){
i = $(this).find('.price');
params[i.attr('name')] = i.val();
i = $(this).find('.lim');
params[i.attr('name')] = i.val();
});
var lnk = $(this);
dojax('/backoffice.php', function (res){
lnk.html('update all');
if (m = res.match(/<!-- U:([^>]*) -->/)) {
updateRows(m[1]);
}
$('.hideit').hide();
$('.tohide').show();
$('.update').text('update');
}, null, null, 'POST', params);
...
function dojax(dourl, afterFunc, hoverCaller, failureFunc, method, params) {
$.ajax({
type: method || 'GET', url: dourl, cache: false,
data: params || null,
global: false,
success: irrelevant
});
}theletterm
New member
Just curious, was this problem solved, or is there still some mystery happening? I have the same issue with my mall alt, and I'd be happy to provide logs is needs be. It looks like y'all have gotten past that point, but I'd love to help in any way I can.
Bale
Minion
The problem is very much not solved.
These days I need to change my prices in the relay browser since mafia won't do that for me anymore.
I would also be glad to volunteer debug logs, but ungawa has taken care of that as well as all the bug reporting I know how to do. I can only hope that one of our brilliant devs will solve this problem once they have time for it and I can make do with the relay browser while I patiently wait.
These days I need to change my prices in the relay browser since mafia won't do that for me anymore.
I would also be glad to volunteer debug logs, but ungawa has taken care of that as well as all the bug reporting I know how to do. I can only hope that one of our brilliant devs will solve this problem once they have time for it and I can make do with the relay browser while I patiently wait.
xKiv
Active member
As far as I can tell this means:
In ManageStoreRequest, change last constructor to
(switch from manageprices to backoffice, update action from update to updateinv, get rid of formInt)
That's it.
Untested, because 1) I am not trying this from *this* computer 2) I don't want to break my mafia anyway 3) I don't have enough items in my store to make a difference, and putting them in would take a lot of time
ETA: perhaps also "this.addFormField( "ajax", "1" );", but I don't know if that requires changes in parsing the response. Response parsing might need changing anyway.
I also don't know if it makes sense to add the pwd and _ "form fields" here. Or if a GET request needs to be forced somehow.
In ManageStoreRequest, change last constructor to
Code:
public ManageStoreRequest( final int[] itemId, final int[] prices, final int[] limits )
{
super( "backoffice.php" );
this.addFormField( "action", "updateinv" );
this.requestType = ManageStoreRequest.PRICE_MANAGEMENT;
for ( int i = 0; i < itemId.length; ++i )
{
this.addFormField( "price" + "[" + itemId[ i ] + "]", prices[ i ] == 0 ? "" : String.valueOf( Math.max(
prices[ i ], Math.max( ItemDatabase.getPriceById( itemId[ i ] ), 100 ) ) ) );
this.addFormField( "limit" + "[" + itemId[ i ] + "]", String.valueOf( limits[ i ] ) );
}
}That's it.
Untested, because 1) I am not trying this from *this* computer 2) I don't want to break my mafia anyway 3) I don't have enough items in my store to make a difference, and putting them in would take a lot of time
ETA: perhaps also "this.addFormField( "ajax", "1" );", but I don't know if that requires changes in parsing the response. Response parsing might need changing anyway.
I also don't know if it makes sense to add the pwd and _ "form fields" here. Or if a GET request needs to be forced somehow.
Last edited:
I also don't have (and don't intend to have) enough items in my store to test this.
xKiv
Active member
I have tried it, with and without ajax=1. In both cases, it seems to properly update prices on the server, but something is still missing - mafia's store manager "clears up" store inventory somewhere along the way, and doesn't fill it back up without seeing the updated store in the response.