ethjs-util icon indicating copy to clipboard operation
ethjs-util copied to clipboard

intToBuffer() fails for numbers that produce odd length hex strings

Open vpulim opened this issue 6 years ago • 13 comments

ethjs-util

Issue Type

  • [X] Bug (https://github.com/ethjs/ethjs-util/blob/master/.github/CONTRIBUTING.md#bug-reports)
  • [ ] Feature (https://github.com/ethjs/ethjs-util/blob/master/.github/CONTRIBUTING.md#feature-requests)

Description

The intToBuffer() function is broken after this PR was merged: https://github.com/ethjs/ethjs-util/pull/7

It fails for numbers that don't produce an even length hex string. It's breaking the following modules:

  • ethereumjs-util: https://github.com/ethereumjs/ethereumjs-util/issues/132
  • ethereumjs-blockchain: https://github.com/ethereumjs/ethereumjs-blockchain/issues/56

Steps to reproduce

const util = require('ethjs-util')

const buf = util.intToBuffer(1);
console.log(buf) // output: "<Buffer >" (expected: "<Buffer 01>")
assert.equal(buf.toString('hex'), '01');

Versions

  • Node/NPM: 9.8.0
  • Browser: N/A

vpulim avatar Jun 08 '18 03:06 vpulim

This is breaking a lot more then just the above mentioned modules. I just wanted to sign a Transaction with ethereumjs-tx, which produced a wrong transaction now! Before converting a hex-string into buffer we always need to make sure the length is even, or new Buffer will throw away these bytes

As a workaround I'm just using now Version 0.1.4 again.

simon-jentzsch avatar Jun 09 '18 09:06 simon-jentzsch

This small bug spoils entire ethereumjs libraries. Please fix this issue as soon as possible. I have already wasted 1 day to fix this bug on my project.

ChandraNakka avatar Jun 10 '18 17:06 ChandraNakka

Will fix in 20 minutes

Sent from my iPhone

On Jun 10, 2018, at 1:22 PM, Chandra Nakka [email protected] wrote:

This small bug spoils entire ethereumjs libraries. Please fix this issue as soon as possible. I have already wasted 1 day to fix this bug on my project.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero avatar Jun 10 '18 18:06 SilentCicero

@vpulim should be up in NPM [email protected] @ChandraNakka @simon-jentzsch

SilentCicero avatar Jun 10 '18 18:06 SilentCicero

@SilentCicero still is not fixed, since lib/index.js still is not changed. Please push new version and unpublish 2 previous versions with npm unpublish. Also please push last changes to repo. This breaks a really lot of things in ETH libraries... @ChandraNakka @simon-jentzsch I'd recommmend use npm/yarn lock file.

fanatid avatar Jun 10 '18 19:06 fanatid

Will fix now and push. Thanks

Sent from my iPhone

On Jun 10, 2018, at 3:42 PM, Kirill Fomichev [email protected] wrote:

@SilentCicero still is not fixed, since lib/index.js still is not changed. Please push new version and unpublish 2 previous versions with npm unpublish. Also please push last changes to repo. This breaks a really lot of things in ETH libraries... @ChandraNakka @simon-jentzsch I'd recommmend use npm/yarn lock file.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero avatar Jun 10 '18 20:06 SilentCicero

@fanatid the module I pushed has the updated lib/index.js code in version 0.1.6:

'use strict';

var isHexPrefixed = require('is-hex-prefixed');
var stripHexPrefix = require('strip-hex-prefix');

/**
 * Pads a `String` to have an even length
 * @param {String} value
 * @return {String} output
 */
function padToEven(value) {
  var a = value; // eslint-disable-line

  if (typeof a !== 'string') {
    throw new Error('[ethjs-util] while padding to even, value must be string, is currently ' + typeof a + ', while padToEven.');
  }

  if (a.length % 2) {
    a = '0' + a;
  }

  return a;
}

/**
 * Converts a `Number` into a hex `String`
 * @param {Number} i
 * @return {String}
 */
function intToHex(i) {
  var hex = i.toString(16); // eslint-disable-line

  return '0x' + hex;
}

/**
 * Converts an `Number` to a `Buffer`
 * @param {Number} i
 * @return {Buffer}
 */
function intToBuffer(i) {
  var hex = intToHex(i);

  return new Buffer(padToEven(hex.slice(2)), 'hex');
}

Can you please expand where the module is failing, please try installing the new version again.

SilentCicero avatar Jun 10 '18 20:06 SilentCicero

Very strange. Sorry, looks like I was on @0.1.5 when tested (i.e. install dependencies for ethereumjs-tx before 0.1.6 was published). Still, can you unpublish 0.1.5 so nobody can use it? Thanks!

fanatid avatar Jun 10 '18 20:06 fanatid

I will depreciate.

SilentCicero avatar Jun 10 '18 20:06 SilentCicero

Not everybody read messages in console, since this library is used in applications which operate with big amounts of money, I think unpublish will be appropriate.

fanatid avatar Jun 10 '18 20:06 fanatid

Okay, will u publish .5 and .4

Sent from my iPhone

On Jun 10, 2018, at 4:53 PM, Kirill Fomichev [email protected] wrote:

Not everybody read messages in console, since this library is used in applications which operate with big amounts of money, I think unpublish will be appropriate.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero avatar Jun 10 '18 22:06 SilentCicero

@SilentCicero Thanks a lot, now it's working :)

ChandraNakka avatar Jun 11 '18 05:06 ChandraNakka

So sorry about that! Clearly not enough test coverage over these basic methods. Will update coverage soon.

Sent from my iPhone

On Jun 11, 2018, at 1:17 AM, Chandra Nakka [email protected] wrote:

@SilentCicero Thanks a lot, now it's working :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

SilentCicero avatar Jun 11 '18 14:06 SilentCicero