auto">Investigating the issue led to this section of code, where we apply updates to the spec on each field specified in the update mask. When we update spec contents we also recompute the size_bytes and hash fields. If the spec is gzipped, we recompute those values using the decompressed spec contents. This also validates that the contents are correctly gzipped.

registry/server/registry/internal/storage/models/spec.go

Lines 166 to 198 in 426d476

    
           for _, field := range mask.Paths { 
        
           	switch field { 
        
           	case "filename": 
        
           		s.FileName = message.GetFilename() 
        
           	case "description": 
        
           		s.Description = message.GetDescription() 
        
           	case "contents": 
        
           		contents := message.GetContents() 
        
           		// if contents are gzipped, uncompress before computing size and hash. 
        
           		if strings.Contains(s.MimeType, "+gzip") && len(contents) > 0 { 
        
           			var err error 
        
           			contents, err = GUnzippedBytes(contents) 
        
           			if err != nil { 
        
           				return status.Error(codes.InvalidArgument, err.Error()) 
        
           			} 
        
           		} 
        
           		s.updateContents(contents) 
        
           	case "mime_type": 
        
           		s.MimeType = message.GetMimeType() 
        
           	case "source_uri": 
        
           		s.SourceURI = message.GetSourceUri() 
        
           	case "labels": 
        
           		var err error 
        
           		if s.Labels, err = bytesForMap(message.GetLabels()); err != nil { 
        
           			return err 
        
           		} 
        
           	case "annotations": 
        
           		var err error 
        
           		if s.Annotations, err = bytesForMap(message.GetAnnotations()); err != nil { 
        
           			return err 
        
           		} 
        
           	} 
        
           }

There is a subtle bug here. The code checks for +gzip in the mime_type of the spec while the spec is being updated. If an update mask specifies contents and mime_type, and the contents field appears earlier in the update mask, then the mime_type check will decide whether to attempt decompression using the incorrect (stale) mime_type.

When an update mask is provided in the request, the mask is normalized before reaching the update code, which sorts the fields and causes contents < mime_type every time - the conditions that lead to an incorrect mime_type check.

registry/server/registry/internal/storage/models/update_masks.go

Lines 55 to 64 in 40ba2cb

    
           func ExpandMask(m protoreflect.ProtoMessage, mask *fieldmaskpb.FieldMask) *fieldmaskpb.FieldMask { 
        
           	if mask == nil || len(mask.GetPaths()) == 0 { 
        
           		return populatedFields(m) 
        
           	} else if len(mask.GetPaths()) == 1 && mask.Paths[0] == "*" { 
        
           		return allFields(m) 
        
           	} 
        
           	mask.Normalize() 
        
           	return mask 
        
           }

Coincidentally, the special cases for update masks use protobuf reflection and seem to create field masks ordered according to the field numbers on the message. This causes mime_type < contents every time - avoiding the conditions that cause the bug.

[name filename description revision_id create_time revision_create_time revision_update_time mime_type size_bytes hash source_uri contents labels annotations]

registry/google/cloud/apigeeregistry/v1/registry_models.proto

Lines 149 to 233 in 40ba2cb

    
           message ApiSpec { 
        
             option (google.api.resource) = { 
        
               type: "apigeeregistry.googleapis.com/ApiSpec" 
        
               pattern: "projects/{project}/locations/{location}/apis/{api}/versions/{version}/specs/{spec}" 
        
             }; 
        
             // Resource name. 
        
             string name = 1; 
        
             // A possibly-hierarchical name used to refer to the spec from other specs. 
        
             string filename = 2; 
        
             // A detailed description. 
        
             string description = 3; 
        
             // Output only. Immutable. The revision ID of the spec. 
        
             // A new revision is committed whenever the spec contents are changed. 
        
             // The format is an 8-character hexadecimal string. 
        
             string revision_id = 4 [ 
        
               (google.api.field_behavior) = IMMUTABLE, 
        
               (google.api.field_behavior) = OUTPUT_ONLY 
        
             ]; 
        
             // Output only. Creation timestamp; when the spec resource was created. 
        
             google.protobuf.Timestamp create_time = 5 [(google.api.field_behavior) = OUTPUT_ONLY]; 
        
             // Output only. Revision creation timestamp; when the represented revision was created. 
        
             google.protobuf.Timestamp revision_create_time = 6 [(google.api.field_behavior) = OUTPUT_ONLY]; 
        
             // Output only. Last update timestamp: when the represented revision was last modified. 
        
             google.protobuf.Timestamp revision_update_time = 7 [(google.api.field_behavior) = OUTPUT_ONLY]; 
        
             // A style (format) descriptor for this spec that is specified as a Media Type 
        
             // (https://en.wikipedia.org/wiki/Media_type). Possible values include 
        
             // "application/vnd.apigee.proto", "application/vnd.apigee.openapi", and 
        
             // "application/vnd.apigee.graphql", with possible suffixes representing 
        
             // compression types. These hypothetical names are defined in the vendor tree 
        
             // defined in RFC6838 (https://tools.ietf.org/html/rfc6838) and are not final. 
        
             // Content types can specify compression. Currently only GZip compression is 
        
             // supported (indicated with "+gzip"). 
        
             string mime_type = 8; 
        
             // Output only. The size of the spec file in bytes. If the spec is gzipped, this is the 
        
             // size of the uncompressed spec. 
        
             int32 size_bytes = 9 [(google.api.field_behavior) = OUTPUT_ONLY]; 
        
             // Output only. A SHA-256 hash of the spec's contents. If the spec is gzipped, this is 
        
             // the hash of the uncompressed spec. 
        
             string hash = 10 [(google.api.field_behavior) = OUTPUT_ONLY]; 
        
             // The original source URI of the spec (if one exists). 
        
             // This is an external location that can be used for reference purposes 
        
             // but which may not be authoritative since this external resource may 
        
             // change after the spec is retrieved. 
        
             string source_uri = 11; 
        
             // Input only. The contents of the spec. 
        
             // Provided by API callers when specs are created or updated. 
        
             // To access the contents of a spec, use GetApiSpecContents. 
        
             bytes contents = 12 [(google.api.field_behavior) = INPUT_ONLY]; 
        
             reserved "revision_tags"; 
        
             reserved 13; 
        
             // Labels attach identifying metadata to resources. Identifying metadata can 
        
             // be used to filter list operations. 
        
             // 
        
             // Label keys and values can be no longer than 64 characters 
        
             // (Unicode codepoints), can only contain lowercase letters, numeric 
        
             // characters, underscores and dashes. International characters are allowed. 
        
             // No more than 64 user labels can be associated with one resource (System 
        
             // labels are excluded). 
        
             // 
        
             // See https://goo.gl/xmQnxf for more information and examples of labels. 
        
             // System reserved label keys are prefixed with 
        
             // "apigeeregistry.googleapis.com/" and cannot be changed. 
        
             map<string, string> labels = 14; 
        
             // Annotations attach non-identifying metadata to resources. 
        
             // 
        
             // Annotation keys and values are less restricted than those of labels, but 
        
             // should be generally used for small values of broad interest. Larger, topic- 
        
             // specific metadata should be stored in Artifacts. 
        
             map<string, string> annotations = 15; 
        
           }

	for _, field := range mask.Paths {
	switch field {
	case "filename":
	s.FileName = message.GetFilename()
	case "description":
	s.Description = message.GetDescription()
	case "contents":
	contents := message.GetContents()
	// if contents are gzipped, uncompress before computing size and hash.
	if strings.Contains(s.MimeType, "+gzip") && len(contents) > 0 {
	var err error
	contents, err = GUnzippedBytes(contents)
	if err != nil {
	return status.Error(codes.InvalidArgument, err.Error())
	}
	}
	s.updateContents(contents)
	case "mime_type":
	s.MimeType = message.GetMimeType()
	case "source_uri":
	s.SourceURI = message.GetSourceUri()
	case "labels":
	var err error
	if s.Labels, err = bytesForMap(message.GetLabels()); err != nil {
	return err
	}
	case "annotations":
	var err error
	if s.Annotations, err = bytesForMap(message.GetAnnotations()); err != nil {
	return err
	}
	}
	}

	func ExpandMask(m protoreflect.ProtoMessage, mask fieldmaskpb.FieldMask) fieldmaskpb.FieldMask {
	if mask == nil \|\| len(mask.GetPaths()) == 0 {
	return populatedFields(m)
	} else if len(mask.GetPaths()) == 1 && mask.Paths[0] == "*" {
	return allFields(m)
	}

	mask.Normalize()
	return mask
	}

	message ApiSpec {
	option (google.api.resource) = {
	type: "apigeeregistry.googleapis.com/ApiSpec"
	pattern: "projects/{project}/locations/{location}/apis/{api}/versions/{version}/specs/{spec}"
	};

	// Resource name.
	string name = 1;

	// A possibly-hierarchical name used to refer to the spec from other specs.
	string filename = 2;

	// A detailed description.
	string description = 3;

	// Output only. Immutable. The revision ID of the spec.
	// A new revision is committed whenever the spec contents are changed.
	// The format is an 8-character hexadecimal string.
	string revision_id = 4 [
	(google.api.field_behavior) = IMMUTABLE,
	(google.api.field_behavior) = OUTPUT_ONLY
	];

	// Output only. Creation timestamp; when the spec resource was created.
	google.protobuf.Timestamp create_time = 5 [(google.api.field_behavior) = OUTPUT_ONLY];

	// Output only. Revision creation timestamp; when the represented revision was created.
	google.protobuf.Timestamp revision_create_time = 6 [(google.api.field_behavior) = OUTPUT_ONLY];

	// Output only. Last update timestamp: when the represented revision was last modified.
	google.protobuf.Timestamp revision_update_time = 7 [(google.api.field_behavior) = OUTPUT_ONLY];

	// A style (format) descriptor for this spec that is specified as a Media Type
	// (https://en.wikipedia.org/wiki/Media_type). Possible values include
	// "application/vnd.apigee.proto", "application/vnd.apigee.openapi", and
	// "application/vnd.apigee.graphql", with possible suffixes representing
	// compression types. These hypothetical names are defined in the vendor tree
	// defined in RFC6838 (https://tools.ietf.org/html/rfc6838) and are not final.
	// Content types can specify compression. Currently only GZip compression is
	// supported (indicated with "+gzip").
	string mime_type = 8;

	// Output only. The size of the spec file in bytes. If the spec is gzipped, this is the
	// size of the uncompressed spec.
	int32 size_bytes = 9 [(google.api.field_behavior) = OUTPUT_ONLY];

	// Output only. A SHA-256 hash of the spec's contents. If the spec is gzipped, this is
	// the hash of the uncompressed spec.
	string hash = 10 [(google.api.field_behavior) = OUTPUT_ONLY];

	// The original source URI of the spec (if one exists).
	// This is an external location that can be used for reference purposes
	// but which may not be authoritative since this external resource may
	// change after the spec is retrieved.
	string source_uri = 11;

	// Input only. The contents of the spec.
	// Provided by API callers when specs are created or updated.
	// To access the contents of a spec, use GetApiSpecContents.
	bytes contents = 12 [(google.api.field_behavior) = INPUT_ONLY];

	reserved "revision_tags";
	reserved 13;

	// Labels attach identifying metadata to resources. Identifying metadata can
	// be used to filter list operations.
	//
	// Label keys and values can be no longer than 64 characters
	// (Unicode codepoints), can only contain lowercase letters, numeric
	// characters, underscores and dashes. International characters are allowed.
	// No more than 64 user labels can be associated with one resource (System
	// labels are excluded).
	//
	// See https://goo.gl/xmQnxf for more information and examples of labels.
	// System reserved label keys are prefixed with
	// "apigeeregistry.googleapis.com/" and cannot be changed.
	map<string, string> labels = 14;

	// Annotations attach non-identifying metadata to resources.
	//
	// Annotation keys and values are less restricted than those of labels, but
	// should be generally used for small values of broad interest. Larger, topic-
	// specific metadata should be stored in Artifacts.
	map<string, string> annotations = 15;
	}

updateSpec does not validate file when mime type is +gzip in the same way as createSpec does #626

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions