Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Correct parameters to str_pad #757

Closed
wants to merge 1 commit into from

Conversation

DavidAnderson684
Copy link

The parameters to str_pad were the wrong way round; see: https://secure.php.net/str_pad

This would cause failures if there were more than 9 blocks. Up until then, the failure was masked.

The parameters to str_pad were the wrong way round; see: https://secure.php.net/str_pad

This would cause failures if there were more than 9 blocks. Up until then, the failure was masked.
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@RobDonovan
Copy link

I dont think that change is correct.

You need to include the STR_PAD_LEFT option in there to make it work correctly.

ie:

$block->setBlockId(base64_encode(str_pad($counter++, 6, '0',STR_PAD_LEFT)));

Also, I cant remember as I haven't looked at it for sometime now, but I think the base64_encode() in that line is incorrect also, so just the fix to str_pad wont fix it 100%. I think it was double base64_encoding the blockID, once on that line and then again within createBlobBlock() just before the send.

@DavidAnderson684
Copy link
Author

STR_PAD_LEFT makes it more elegant and intuitive (it'll still be a unique ID without).

I haven't checked the base64 encoding. I'm not using this method myself. I just noticed the str_pad error whilst comparing my own chunked code with this fragment in the SDK. So probably best if I leave it to you, if you actively use the method. I just thought it'd be helpful to flag up that the current code can't work at all for people with >9 blocks.

@RobDonovan
Copy link

I think the STR_PAD_LEFT is needed to keep the blockID unique

Since:

echo str_pad(1, 6, '0') ."\n";
echo str_pad(100, 6, '0') ."\n";

Without STR_PAD_LEFT, the result of both of these is '100000', which is not unique

@DavidAnderson684
Copy link
Author

You are correct - sorry; I am rushing too much.

@devigned devigned closed this Dec 17, 2015
@devigned devigned reopened this Dec 17, 2015
@azurecla
Copy link

Hi @DavidAnderson684, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no contribution license agreement is required at this point. Real humans will now evaluate your PR.

TTYL, AZPRBOT;

yaqiyang pushed a commit to yaqiyang/azure-sdk-for-php that referenced this pull request Mar 17, 2016
@yaqiyang
Copy link
Member

Since @RobDonovan did not provide a PR, I pushed a fix to https://github.com/yaqiyang/azure-sdk-for-php/tree/pull757

Thanks
Yaqi

@yaqiyang yaqiyang closed this Mar 17, 2016
yaqiyang pushed a commit to yaqiyang/azure-sdk-for-php that referenced this pull request Mar 18, 2016
yaqiyang added a commit that referenced this pull request Mar 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants