invalid .proto generation

[i007c]

this code uses an invalid id for fields thus resulting in an invalid .proto .schema

rawproto/src/index.js

Line 492 in 795b15f

for (const n in treekeys) {

it should be something like this

for (const [fn, f] of Object.entries(tree.sub || {}))

[konsumer]

@i007c can you give an example this fails on? I think proto generation is a bit flakey, but it's a dark art (protobuf is not meant to be examined this way) so seeing an example would help.

[i007c]

this code is just invalid. the in keyword in javascript doesnt work like in python you have to use the of keyword.

The in operator returns true if the specified property is in the specified object or its prototype chain.

i cant upload a pbf file in here but i was decoding google maps vector tiles that i ran into this issue.
clone the repo and ran on localhost with this fix

I think proto generation is a bit flakey
yes even with this fix its kind of flakey. but half-decent is better than broken

the main problem was that field ids where just indexed on after another not the actual id of the field

[konsumer]

this code is just invalid. the in keyword in javascript doesnt work like in python you have to use the of keyword.

You are wrong. The n here is a numeric-key of the keys of the object. See here. In this particular place, it's a string (because strings are use as keys in objects, and it gets used in a couple different ways) so I parseInt it, to be sure. Did you log it? It does work.

if you look closer at that code, in context, you will see I am doing this:

const treekeys = Object.keys(tree.sub || {})

for (const n in treekeys) {
   const fn = parseInt(n) + 1
   const f = tree?.sub[treekeys[n]]
}

This is not "incorrect" but could be done with entries too, if I wanted to:

for (const [n, f] of Object.entries(tree.sub || {})) {
   const fn = parseInt(n) + 1
}

I don't because I use the keys separately in a few places, so this is slightly more efficient.

Your code has a subtle problem of fn being an off-by-1 int string, which means it doesn't operate with math like you would expect (for example if fn is 1 and you add 1, it will be "11".)

i cant upload a pbf file in here but i was decoding google maps vector tiles that i ran into this issue.
clone the repo and ran on localhost with this fix

This is why I want to see an example. I suspect it's some other problem for you. I am not saying my code never has issues, just that in is not it, here, as far as I can tell. Like I said, it's a very dark art, and often this stuff doesn't work exactly like protobuf's documented, which means I tune this code by test-cases.

The original code here came from an old project of mine, that worked in production (on lots of systems) for a long time. I made some major changes to how this library works, so I actually expect some things to be off, until I have tested it enough.

I have passing unit-tests for this part. If you can provide a failing test-case, I am willing to look closer at what you have to say about it, but otherwise what I hear is just "this code is invalid", and you are wrong about that, so why should I care?

"Works on my machine but I can't replicate or show a test case" is not going to work for me. You can't generate some binary protobuf that shows the issue? You said the data where you found the issue isn't made by you anyway (google maps?) Send some binary protobuf data from some random map-place, unrelated to whatever you are doing, that shows the problem.

the main problem was that field ids where just indexed on after another not the actual id of the field

protobuf always has numeric index-based keys. That is how it works. The fieldnames normally come from the proto SDL, if you have that, but in the binary, it's all ints. This code here derives the unknown field-name from the field-number, but the field-number is set in stone, embedded in the data.

[konsumer]

actually, scratch what I said about Object.entries, even this code is wrong there:

for (const [n, f] of Object.entries(tree.sub || {})) {
   const fn = parseInt(n) + 1
}

The purpose of that code is different than you are assuming, I think. fn is just "an incrementing integer" to use in message/field name, if there is no typemap. I think I would need to increment an int, maybe something like this:

let fn = 1
for (const [n, f] of Object.entries(tree.sub || {})) {
  // stuff
  fn++
}

but my point still stands, there is nothing wrong with how I am doing it, and it saves a couple lines, and is slightly more efficient. Your code would not work as expected, so i am not sure why I am even talking about it.

The original code here was made before Object.entries existed (back when you have to use keys/values) but since the treekeys is used multiple times, it's still more efficient to do it how I am doing it.

[i007c]

bruv what are you talking about

for (const n in Object.keys({1: 'one', 4: 'four'})) {
    console.log(parseInt(n) + 1)
}

this code output is

1
2

which is incorrect for what we want. it does not give us the field id its only gives index which is invalid for the purpose if generating proto schemas

[i007c]

You are wrong. The n here is a numeric-key of the keys of the object

yes but you are using it on a array not the tree.sub object

[konsumer]

bruv what are you talking about

What are you talking about? Again, I have passing tests for proto-generation. If you can give me a test that shows it's not doing what it's supposed to, or even just describe a use-case that fails, then I have something to look at, but this does work fine for me.

yes but you are using it on a array not the tree.sub object

I don't know what you mean. Yes, this is an array, I get it on the line above, like this: Object.keys(tree.sub || {})

which is incorrect for what we want. it does not give us the field id its only gives index which is invalid for the purpose if generating proto schemas

That is not what we want here. What we want is an individual number for Mesage#, that correlates correctly with the field and the typemap-path, which this does fine.

It operates on other stuff you're not accounting for, like typemaps. If you can produce a PR with a working test, then I still need to see how it's better to accept it, but you haven't even done this. You haven't even clearly stated what problem you're having with how it works now. How is the proto "invalid" in your case?

I suspect this is an XY problem, but it's impossible to know for sure or know how to help without more info. What are you trying to do, and how does this library not do it? What have you tried so far?