The Magento web services API is quite flexible - a lot of the time, it can either take an ID (like a product ID) or a more human-readable identifier (like a product SKU). Unfortunately, the downside of this is that in certain circumstances these can be mixed up, in which case the the wrong product will be affected.

In theory, to fetch information on a product based on its ID, the catalog_product.info method should be used with with an integer argument:

$result = $client->call($sessionId, 'catalog_product.info', 124);

If you wanted to fetch by SKU on the other hand, you would provide a string argument:

$result = $client->call($sessionId, 'catalog_product.info', 'big-hat');

But a problem can occur if you're using products with numeric SKUs that are also valid product ID numbers. For example, if you have a product with the SKU '120' (as a string), and you also have an existing product with the ID 120, there's the potential for confusion. The core code that actually gets the product when you make any of the catalog_product web services API calls is (from app/code/core/Mage/Catalog/Model/Api/Resource.php):

    protected function _getProduct ($productId, $store = null)
    {
        $product = Mage::getModel('catalog/product');
 
        if (is_string($productId)) {
            $idBySku = $product->getIdBySku($productId);
            if ($idBySku) {
                $productId = $idBySku;
            }
        }
        if ($store !== null) {
            $product->setStoreId($this->_getStoreId($store));
        }
        $product->load($productId);
        return $product;
    }

The issue is that if this function is passed a string as its $productId argument, it will first try to find the product with a SKU matching that string. If it fails, though, it doesn't return with an error that it couldn't find anything - it tries to find a product with that string as an ID. In the above example, if a product is already in the system with ID 120, and you want to know if a product with the SKU '120' exists, you would try:

$result = $client->call($sessionId, 'catalog_product.info', '120');

Note that we're passing '120' as a string. This should search for any products with the SKU '120', and since there aren't any it should return an error. However, since there is a product with the ID 120, and since '120' is the same as 120 if you cast it to an integer, you'll instead get the details for this product back instead.

This isn't too bad if we're just trying to get information on a product - we can always test manually whether the SKU in the array returned by catalog_product.info is the same as the SKU we searched for. But if we are instead performing an update operation, the result can be subtle yet very severe. If you try to update a product with the SKU '120' and it doesn't exist, you should get an error. Instead, if there happens to be a product with the ID 120, that will be updated instead, which may well go unnoticed should IDs and SKUs only occasionally match up. Nasty.

One solution to this problem is to override the block code above with a version that doesn't fall back to searching on ID if it can't find a match on SKU:

    protected function _getProduct ($productId, $store = null)
    {
        $product = Mage::getModel('catalog/product');
        if ($store !== null) {
            $product->setStoreId($this->_getStoreId($store));
        }
 
        if (is_string($productId)) {
            $idBySku = $product->getIdBySku($productId);
            if ($idBySku) {
                $product->load($idBySku);
            }
        }
        else
        {
            $product->load($productId);
        }
        return $product;
    }

Another option is just to be extremely defensive and double-check everything. When using catalog_product.info, check that what you get back really is what you're looking for, and don't do any updates until you've confirmed they will affect the right product.

Update: Moved return statement as suggested by Anton. We have also received word that this issue won't be fixed in upcoming Magento core releases for now, since an API overhaul is apparently already in development. The expectation is that the new API included with a future major release will support either separate methods of getting products by SKU or by ID, or a way of explicitly specifying how to treat the productId argument.

Comments

Good job!
May we include this code into Magento? ;)

$idBySku = $product->getIdBySku($productId);
if ($idBySku) {
$product->load($idBySku);
}
return $product;

What happens if you don't get value $idBySku? Bug.

The return $product; can be moved to the end of the method to not duplicate it.

Thanks Anton, we'd love to see this bug fixed in Magento and would gladly welcome the assistance of anyone who can make that happen!

"What happens if you don't get value $idBySku? Bug."

The only difference in behaviour I can see between our version and the original is that the original will always call load() on the product object, even if it calls it with an invalid product ID. This adds some additional empty arrays to the product object (i.e. stock_item, media_gallery, etc), but since we want to return from whatever API call was used with a 'product doesn't exist' error anyway, I don't think there is any appreciable difference. The behaviour should be basically the same as before, in that an empty product object is returned. Our aim was to change as little as possible while still addressing the "fall-through" error.

I've noticed that deleting a non-existant sku like 333whatever, will delete product id 333.

Haven't checked the source code yet, but I assume php is casting 333whatever to an integer, which gives 333.

This is while using the API anyway.

Cheers

Heh, that's totally hilarious. What version, so I can avoid it if possible?

Our SKU looks like 14-00001. If the product with that sku does not exists the magento returns
the product with the ID 14. The following change does work perfekt for us.

protected function _getProduct($productId, $store = null, $identifierType = null)
{
$loadByIdOnFalse = false;
if ($identifierType == null) {
$identifierType = 'sku';
if (is_numeric($productId)) {
$loadByIdOnFalse = true;
}
}
 
$product = Mage::getModel('catalog/product');
if ($store !== null) {
$product->setStoreId($this->_getStoreId($store));
}
/* @var $product Mage_Catalog_Model_Product */
if ($identifierType == 'sku') {
$idBySku = $product->getIdBySku($productId);
if ($idBySku) {
$productId = $idBySku;
}
if ($idBySku || $loadByIdOnFalse) {
$product->load($productId);
}
} elseif ($identifierType == 'id') {
$product->load($productId);
}
return $product;
}