-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TS]: builder, Fix requiredField(). Verity that the field is present in the vtable (#7739) #7752
[TS]: builder, Fix requiredField(). Verity that the field is present in the vtable (#7739) #7752
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
@dbaileychess, thanks for the review. All suggestions have been applied. I must say that this same problem happens in C++. And thus, maybe everywhere. I've checked the C++ side and even thought the language is not a barrier for me, I didn't find a proper way to apply the fix. How to proceed here, considering all languages are susceptible to have this issue? |
Verifty that the field is present in the vtable.
As said here this was a wrong assumption and C++ is not affected by this bug. |
Issue description updated. The problem was not the buffer creation but a missing check (whether the field offset is in the bounds of the vtable) in |
This PR is ready for review @dbaileychess. |
Hi, is there an estimated time for a new release containing this fix? |
…in the vtable (google#7739) (google#7752) * [TS]: Fix vtable creation for consecutive required fileds (google#7739) * handle feedback * comment the schema * comment change in builder.ts * [TS]: builder, Fix requiredField() Verifty that the field is present in the vtable. * restore monsterdata binary file Co-authored-by: Derek Bailey <[email protected]>
…in the vtable (google#7739) (google#7752) * [TS]: Fix vtable creation for consecutive required fileds (google#7739) * handle feedback * comment the schema * comment change in builder.ts * [TS]: builder, Fix requiredField() Verifty that the field is present in the vtable. * restore monsterdata binary file Co-authored-by: Derek Bailey <[email protected]>
For the example given in the bug report (#7739)
CreateX
would not throw when such a missing 'required' field is not present.This bug defeats the strong typing purpose since the creation of a buffer that should throw due to not fulfilling the 'required' flag would indeed succeed and crash when accessing such non present field.
The problem is within
builder.requiredField()
method which does not check that the field offset is within the bounds of the vtable. This PR fixes exactly that.