Attribute safety

[Brian Goetz]

Just a further thought on this: we can further focus our lens on _bound_ 
attributes, because these are the ones that have come from another 
classfile.  If the user creates a RVAA during a transform, we should 
assume that is fine, just as we do with writing.

On 8/7/2023 6:46 AM, Adam Sotona wrote:
>
> That makes perfect sense, attribute safety is more exactly an 
> attribute transformation safety.
>
> I agree that introduction of special read/write filters (in a form of 
> context options) is confusing and non-systematic.
>
> When we focus on the implementation of the attributes transformation 
> safety, I think the safety switch is less of a global context option 
> but rather individual transformation immediate feature (a filtering 
> feature).
>
> If we implement it as a global context option, we would have to insert 
> a filtering layer before each transformation (on read side) or after 
> (on write side) of each transformation. I think it would be pretty 
> much the same as filtering on read/write, except for the fact it will 
> affect transformations only (so maybe even more confusing). 
> Classfile::transform would then behave differently than its expanded 
> form using Classfile::build.
>
> However if we implement attribute transformation safety as specific 
> transformations (doing the filtering job) – it should work in harmony 
> with the rest of the API.
>
> For example in addition to ClassTransform.ACCEPT_ALL we can add 
> ClassTransform.ACCEPT_ALL_KNOWN_ATTRIBUTES (dropping UNKNOWN) and 
> ClassTransform.ACCEPT_ALL_SAFE_ATTRIBUTES (dropping HAZMAT).
>
> As an interesting expansion of the ClassfileTransform features we can 
> provide factories like for example 
> ClassfileTransform::dropingAll(Predicate<ClassfileElement> filter) – 
> where the “All” (or “Deep” or similar suffix) should indicate forced 
> expansion of the whole tree, so the filter is really applied on all 
> levels and filtered element never appears in the target class. It can 
> be used to implement the global filtering transformations.
>
> I propose to add following set of filtering transformations:
>
>   * ClassTransform.ACCEPT_ALL_KNOWN_ATTRIBUTES
>   * ClassTransform.ACCEPT_ALL_SAFE_ATTRIBUTES
>   * FieldTransform.ACCEPT_ALL_KNOWN_ATTRIBUTES
>   * FieldTransform.ACCEPT_ALL_SAFE_ATTRIBUTES
>   * MethodTransform.ACCEPT_ALL_KNOWN_ATTRIBUTES
>   * MethodTransform.ACCEPT_ALL_SAFE_ATTRIBUTES
>   * CodeTransform.ACCEPT_ALL_KNOWN_ATTRIBUTES
>   * CodeTransform.ACCEPT_ALL_SAFE_ATTRIBUTES
>
> Thanks,
>
> Adam
>
> *From: *Brian Goetz <brian.goetz at oracle.com>
> *Date: *Saturday, 5 August 2023 3:01
> *To: *Adam Sotona <adam.sotona at oracle.com>, 
> classfile-api-dev at openjdk.org <classfile-api-dev at openjdk.org>
> *Subject: *Re: Attribute safety
>
> OK, I thought about this some more while sitting in the sauna ...
>
> I think the locus of attribute safety is not reading or writing, but 
> transforming.  If I am just going to read a classfile, there is no 
> need to drop anything, if I find an attribute I don't recognize, I'll 
> just skip over it and keep going -- that's how attributes are designed 
> to work.  No need to drop anything on read, ever.  WHen the library 
> finds an unknown attribute, it wraps it with an UnknownAttribute 
> element, whose understanding to the attribute is limited to name, 
> size, and byte[] of the payload.  Nothing so dangerous here that the 
> user needs protection.
>
> Similarly, if a user is _writing_ a classfile, again, we should trust 
> them that the classfile they are putting together is sensible.  We 
> shouldn't second guess at "oh, that's a type annotations attribute, 
> those are so brittle, please sign here."
>
> Where there is potentially a problem is when we are _transforming_ a 
> classfile, because for a HAZMAT or UNKNOWN attribute, we can't 
> guarantee its integrity if we've changed anything else about the 
> classfile (including reordering the constant pool.)  So the "what do 
> we do with brittle attributes" question applies only to 
> transformation, where we are taking an attribute from one classfile (a 
> bound attribute) and writing it to another.  This is where the user 
> can shoot themselves in the foot, because they might change something 
> else about the classfile and subtly (or not subtly) undermine the 
> integrity of the annotation they don't understand.  And this is why we 
> want to classify attributes according to their sensitivity to 
> environmental change:
>
>   - A stateless attribute is sensitive to no environmental changes.  A 
> transform can always safely bulk-copy the attribute directly.
>
>   - An attribute with CP dependencies is sensitive to restructuring of 
> the constant pool (no CP sharing), but the mapper contains enough 
> information to survive CP restructuring.  A transform can safely 
> bulk-copy the attribute directly if the CP is shared between the 
> original and new classfile, and can otherwise safely copy the 
> attribute by inflating it and deflating it via the 
> readAttribute/writeAttribute behavior of the mapper.
>
>  - An attribute with label dependencies is sensitive to changes to the 
> contents or structure of the bytecode array.  A transform can safely 
> bulk-copy the attribute directly if code array is unchanged, and can 
> otherwise safely copy the attribute by inflating it and deflating it 
> via the readAttribute/writeAttribute behavior of the mapper.  However, 
> there are currently no attributes that have label dependencies only 
> but are not already treated specially by the classfile API, so this 
> category may not be that interesting.
>
>  - An attribute with unpredictable dependencies is sensitive to any 
> change to the contents of the entity of which it is an attribute.  It 
> can be safely bulk-copied if nothing else in that entity has changed, 
> but otherwise there is no safe way to copy it.
>
>  - An unknown attribute is sensitive to all of the above, and so takes 
> on the union of the copying risks of all of the above.
>
> So I think the Option we want governs what to do with various 
> attributes when _transforming_ a CompoundElement in which they 
> appear.  And the problematic cases are those with unpredictable 
> dependencies, and unknowns.  So I think the options we want are:
>
>  - When transforming, always keep HAZMAT and UNKNOWN attributes; for 
> safety, lift and lower HAZMAT attributes.
>  - When transforming, keep HAZMAT attributes (lifting and lowering), 
> but always drop UNKNOWN attributes.
>  - When transforming, always drop HAZMAT and UNKNOWN attributes.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/classfile-api-dev/attachments/20230823/f89c4716/attachment-0001.htm>