[hi5 -> IL] SetPrivateStoreListBuy issue

[Tryskell]

Hi all, I try my luck here, let's say if someone talented can answer.

I try to adapt shops packets. SetPrivateStoreListBuy is one of them, and I got stupid issue with it. _items stays null with the current code, and for so, blocks at the current part of code :

Code: Select all

        if (_items == null)        {            player.setPrivateStoreType(L2PcInstance.STORE_PRIVATE_NONE);            player.broadcastUserInfo();            player.sendPacket(new PrivateStoreManageListBuy(player));            return;        }

About trollers, SetPrivateStoreListSell has been correctly reworked and so far works perfectly. So what I ask is POSSIBLE and can WORK. Still about trollers, this is Legacy section so don't say "Interlude is shit, update Freya".

Code: Select all

/* * This program is free software: you can redistribute it and/or modify it under * the terms of the GNU General Public License as published by the Free Software * Foundation, either version 3 of the License, or (at your option) any later * version. *  * This program is distributed in the hope that it will be useful, but WITHOUT * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more * details. *  * You should have received a copy of the GNU General Public License along with * this program. If not, see <http://www.gnu.org/licenses/>. */package net.sf.l2j.gameserver.network.clientpackets; import net.sf.l2j.Config;import net.sf.l2j.gameserver.model.TradeList;import net.sf.l2j.gameserver.model.actor.L2Character;import net.sf.l2j.gameserver.model.actor.instance.L2PcInstance;import net.sf.l2j.gameserver.network.SystemMessageId;import net.sf.l2j.gameserver.network.serverpackets.PrivateStoreManageListBuy;import net.sf.l2j.gameserver.network.serverpackets.PrivateStoreMsgBuy;import net.sf.l2j.gameserver.network.serverpackets.SystemMessage;import net.sf.l2j.gameserver.taskmanager.AttackStanceTaskManager; public final class SetPrivateStoreListBuy extends L2GameClientPacket{    private static final int BATCH_LENGTH = 12; // length of one item        private Item[] _items = null;     @Override    protected void readImpl()    {        int count = readD();        if (count < 1 || count > Config.MAX_ITEM_IN_PACKET || count * BATCH_LENGTH != _buf.remaining())            return;                _items = new Item[count];        for (int i = 0; i < count; i++)        {            int itemId = readD();            readH(); //TODO analyse this            readH(); //TODO analyse this            long cnt = readD();            int price = readD();                        if (itemId < 1 || cnt < 1 || price < 0)            {                _items = null;                return;            }            _items[i] = new Item(itemId, (int)cnt, price);        }    }     @Override    protected void runImpl()    {        L2PcInstance player = getClient().getActiveChar();        if (player == null)             return;         if (_items == null)        {            player.setPrivateStoreType(L2PcInstance.STORE_PRIVATE_NONE);            player.broadcastUserInfo();            player.sendPacket(new PrivateStoreManageListBuy(player));            return;        }         if (!player.getAccessLevel().allowTransaction())        {            player.sendPacket(new SystemMessage(SystemMessageId.YOU_ARE_NOT_AUTHORIZED_TO_DO_THAT));            return;        }                if (AttackStanceTaskManager.getInstance().getAttackStanceTask(player)            || (player.isCastingNow() || player.isCastingSimultaneouslyNow())            || player.isInDuel())        {            player.sendPacket(new SystemMessage(SystemMessageId.YOU_ARE_NOT_AUTHORIZED_TO_DO_THAT));            player.sendPacket(new PrivateStoreManageListBuy(player));            return;        }                if (player.isInsideZone(L2Character.ZONE_NOSTORE))        {            player.sendPacket(new SystemMessage(SystemMessageId.NO_PRIVATE_STORE_HERE));            player.sendPacket(new PrivateStoreManageListBuy(player));            return;        }         TradeList tradeList = player.getBuyList();        tradeList.clear();         // Check maximum number of allowed slots for pvt shops        if (_items.length > player.getPrivateBuyStoreLimit())        {            player.sendPacket(new SystemMessage(SystemMessageId.YOU_HAVE_EXCEEDED_QUANTITY_THAT_CAN_BE_INPUTTED));            player.sendPacket(new PrivateStoreManageListBuy(player));            return;        }                int totalCost = 0;        for (Item i : _items)        {            if (!i.addToTradeList(tradeList))            {                player.sendPacket(new SystemMessage(SystemMessageId.EXCEEDED_THE_MAXIMUM));                player.sendPacket(new PrivateStoreManageListBuy(player));                return;            }                        totalCost += i.getCost();            if (totalCost > Integer.MAX_VALUE)            {                player.sendPacket(new SystemMessage(SystemMessageId.EXCEEDED_THE_MAXIMUM));                player.sendPacket(new PrivateStoreManageListBuy(player));                return;            }        }         // Check for available funds        if (totalCost > player.getAdena())        {            player.sendPacket(new SystemMessage(SystemMessageId.THE_PURCHASE_PRICE_IS_HIGHER_THAN_MONEY));            player.sendPacket(new PrivateStoreManageListBuy(player));            return;        }         player.sitDown();        player.setPrivateStoreType(L2PcInstance.STORE_PRIVATE_BUY);        player.broadcastUserInfo();        player.broadcastPacket(new PrivateStoreMsgBuy(player));    }     private static class Item    {        private final int _itemId, _count, _price;                public Item(int id, int num, int pri)        {            _itemId = id;            _count = num;            _price = pri;        }                public boolean addToTradeList(TradeList list)        {            if ((Integer.MAX_VALUE / _count) < _price)                return false;                        list.addItemByItemId(_itemId, _count, _price);            return true;        }                public long getCost()        {            return _count * _price;        }    }     @Override    public String getType()    {        return "[C] 91 SetPrivateStoreListBuy";    }}

I compared with IL (if I didn't missthink packet format on the read part), with hi5, with SetPrivateStoreListSell (both packets are clone of each other, except on the part to add item ; where one use itemId, other uses objectID). I even sniffed packets and compared both versions from L2J and L2OFF, and they send the same crap.

But the _items section stays null. If I remove the null check I got a NPE (that sounds logical, but that just to say it really blocks on that section).

Here are the others versions of that code : Interlude | Current unstable

Pastebin of my current version : http://pastebin.com/VUnHrk2j

Ty in advance if you find anything .

Tk.

PS : I tested with one item/multiple items, and with different quantities aswell. The _lists still stay null. But packet is correctly send with the good infos.

[jurchiks]

Doesn't readD() return int? If so, then "cnt" should be datatype=int.
Also, have you tried adding some debug/logging there to see the params?
For example, first dump the _items list before the for loop, and then each item params inside the loop.
Also, perhaps, instead of this:
_items = null;
return;
you should consider just "continue"?

[Tryskell]

Ty jurchiks for the try .

- readD is effectively an int normally. I first made it an int, then following L2J IL I decided to revert back to long, without effect. It currently works as a long => int and worked too before as an simple int on SetPrivateStoreListSell.
- so far, Item[] is like a bag, where you put one or more Item(param1, param2, param3) inside. Each Item got a "slot" ("i" value) allocated in that bag Item[]. If one of the 3 params of Item is wrong, then Item is just forgotten. As you can see it returns in the loop, which means if there are more Item, they're not dropped, even if one Item was wrong before. Btw if there was an issue on that part, L2J would have too .

I just tested both of your solutions (revert long to int, and return to continue), and same bug .

Ty to cared about my problem ^^. I'm rarely stuck like that. I will try to log variables.

PS : I just reverted the whole change of that serverpacket (I kept on diff patch), and old code is working. It's trully the current implementation of that packet which make it buggy.

PS2 :if you ask why to not use old code, I would answer : Item[] is more sexy . No reason it works for 5 others packets (procure buy/sell, request buy/sell, and shop sell) and not for this one (shop buy)...

[Tryskell]

I finally found where it was buggy (and I'm not even sure it's very good about security).

Code: Select all

count * BATCH_LENGTH != _buf.remaining()

=>

Code: Select all

count * BATCH_LENGTH > _buf.remaining()

Changing != for > resolves the problem.

Now the questions :
- when the "fix"is basically revert back to L2J IL, why does it currently work with SetPrivateStoreListSell setting != ? Why doesn't it bug too ?
- Is there any security threat ? I understand it's about packet length.
- Does that mean BATCH_LENGTH got a bad number ? On IL it was setup to 12, on hi5 it's 40. If it's based on infos like I think, does it should count like that = 4(D)+2(H)+2(H)+4(D)+4(D) = 16 ?

PS : fock me, I was right . Fockin BATCH_LENGTH is 16, not 12 (as says IL). It currently works with !=. Who the hell did that packet, lol. I must craft a voodoo doll...

Ty a lot jurchiks, I really appreciated your help . That helped me to focus on the top of the code.

So to resume : at the top of the readImp1 part, there's a check about the packet length. If count of items * length of items is different of length of total packet, it was returning null (can it be loggable ? I don't even think you can log in readImp1).

So my stupid packet was sending good infos, but the check just higher basically dropped the information... Gl to track that.

[jurchiks]

Shit happens
Anyway, glad you found the problem