Issue with packed values with tag > 15

[cschwarz]

After updating to pbf 3.x I have issues reading packed values with a tag number greater than 15.

syntax = "proto3";
package ProtobufTest;

message Working {   
    repeated uint32 values = 15;
}

syntax = "proto3";
package ProtobufTest;

message NotWorking {    
    repeated uint32 values = 16;
}

The problem seems to be in readPackedEnd which guesses the type based on pbf.pos -1.

function readPackedEnd(pbf) {
    return (pbf.buf[pbf.pos - 1] & 0x7) === Pbf.Bytes ?
        pbf.readVarint() + pbf.pos : pbf.pos + 1;
}

[kjvalencik]

Confirmed this issue and the location. While clearly not a proper fix, adding the following resolves the issue.

Breaking PR: https://github.com/mapbox/pbf/pull/57/files
Breakign commit: 79dfc1a

diff --git a/index.js b/index.js
index 17d2f0f..f532853 100644
--- a/index.js
+++ b/index.js
@@ -34,6 +34,7 @@ Pbf.prototype = {
                 tag = val >> 3,
                 startPos = this.pos;

+            global.__val = val;
             readField(tag, result, this);

             if (this.pos === startPos) this.skip(val);
@@ -393,7 +394,7 @@ function readVarintRemainder(l, s, p) {
 }

 function readPackedEnd(pbf) {
-    return (pbf.buf[pbf.pos - 1] & 0x7) === Pbf.Bytes ?
+    return (global.__val & 0x7) === Pbf.Bytes ?
         pbf.readVarint() + pbf.pos : pbf.pos + 1;
 }

Previously, I had a PR that passed the type through but, this was a breaking change to the interface. I'll take another look to see if there is a better way to get the type.

EDIT

This bug doesn't strictly effect all ids over 15. It only effects numbers where the type is split across the last two bytes instead of the last byte.

[kjvalencik]

@mourner This is the only fix I can come up with that doesn't change the API to pass type around. Thoughts?

diff --git a/index.js b/index.js
index 17d2f0f..a60985e 100644
--- a/index.js
+++ b/index.js
@@ -7,6 +7,7 @@ var ieee754 = require('ieee754');
 function Pbf(buf) {
     this.buf = ArrayBuffer.isView(buf) ? buf : new Uint8Array(buf || 0);
     this.pos = 0;
+    this.type = 0;
     this.length = this.buf.length;
 }

@@ -34,6 +35,7 @@ Pbf.prototype = {
                 tag = val >> 3,
                 startPos = this.pos;

+            this.type = val & 0x7;
             readField(tag, result, this);

             if (this.pos === startPos) this.skip(val);
@@ -393,7 +395,7 @@ function readVarintRemainder(l, s, p) {
 }

 function readPackedEnd(pbf) {
-    return (pbf.buf[pbf.pos - 1] & 0x7) === Pbf.Bytes ?
+    return pbf.type === Pbf.Bytes ?
         pbf.readVarint() + pbf.pos : pbf.pos + 1;
 }

[mourner]

I think that's even better than the previous implementation. I'll do a patch release tomorrow. @kjvalencik thanks for reacting so quickly!

[mourner]

Released as 3.0.2.

[cschwarz]

Thanks for the fast bug fix and release!