Skip to content

Fix fragment-offset-range pattern escaping in single-quoted strings#1460

Open
charanjith-anet wants to merge 3 commits into
openconfig:masterfrom
charanjith-anet:fix-fragment-offset-pattern
Open

Fix fragment-offset-range pattern escaping in single-quoted strings#1460
charanjith-anet wants to merge 3 commits into
openconfig:masterfrom
charanjith-anet:fix-fragment-offset-pattern

Conversation

@charanjith-anet
Copy link
Copy Markdown

Fix typo in fragment-offset-range pattern: '\.' should be '.'

In single-quoted YANG strings, there is no escape processing (RFC 7950 §6.1.3). So '\.' is stored as two backslashes followed by a dot, which in regex matches "backslash then any char" — not "literal dot".

Every other OC pattern that needs a literal dot uses '.' correctly,
e.g. area-address in openconfig-isis-types.yang:
pattern '[a-fA-F0-9]{2}(.[a-fA-F0-9]{4}){3,9}.[a-fA-F0-9]{2}';

Change Scope

  • \\.\. in both pattern and posix-pattern for fragment-offset-range
  • Backwards compatible

Platform Implementations

Pattern-only fix, no model/tree changes.

Tree View

N/A

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the fragment-offset-range regex pattern in openconfig-packet-match-types.yang by replacing double backslashes with single backslashes for the range separator. The reviewer suggests adding regression tests to a separate test file to validate the pattern against various valid and invalid inputs.

Comment thread release/models/acl/openconfig-packet-match-types.yang
The pattern and posix-pattern for fragment-offset-range use '\\.'
(double backslash) in single-quoted YANG strings. Since single-quoted
strings have no escape processing (RFC 7950 Section 6.1.3), '\\.'
stores as literal backslash-backslash-dot, which in regex means
"literal backslash followed by any character" — not the intended
"literal dot".

All other patterns in the OpenConfig models correctly use '\.' (single
backslash) for literal dot matching in single-quoted strings (e.g.
area-address in openconfig-isis-types.yang).
@charanjith-anet charanjith-anet force-pushed the fix-fragment-offset-pattern branch from 4194a37 to 4c53a6b Compare March 26, 2026 21:24
@dplore dplore moved this to Ready to discuss in OC Operator Review Apr 8, 2026
@dplore
Copy link
Copy Markdown
Member

dplore commented Apr 8, 2026

/gcbrun

@OpenConfigBot
Copy link
Copy Markdown

OpenConfigBot commented Apr 8, 2026

No major YANG version changes in commit eed2855

@ElodinLaarz
Copy link
Copy Markdown
Contributor

/gcbrun

@ElodinLaarz
Copy link
Copy Markdown
Contributor

Reviewed at the OC Operators Meeting on May 5th, 2026:

If the tests are passing, they seem to verify this is working as intended after the change.

Seems to match the formatting used in, e.g. this pattern.

We'll move to last-call in 2 weeks assuming the tests pass. (May 19th, 2026)

Copy link
Copy Markdown
Contributor

@ElodinLaarz ElodinLaarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update OC version since we're editing the file.

Comment on lines 26 to 32
oc-ext:openconfig-version "1.3.4";

revision "2025-06-10" {
description
"Add pattern regex for Fragment Offset range";
reference "1.3.4";
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update the version number.

Suggested change
oc-ext:openconfig-version "1.3.5";
revision "2026-05-12" {
description
"Fix pattern regex for Fragment Offset range";
reference "1.3.5";
}

Copy link
Copy Markdown
Member

@dplore dplore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to update version number, but otherwise LGTM.

Reviewed in May 12, 2026 OC Operators meeting without objection.

@dplore dplore moved this from Ready to discuss to last-call in OC Operator Review May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: last-call

Development

Successfully merging this pull request may close these issues.

4 participants