Skip to content

Reject non-ASCII digits in vWeekday ordwk (BYDAY/BYWEEKDAY/WKST)#1488

Open
Labib-Bin-Salam wants to merge 1 commit into
collective:mainfrom
Labib-Bin-Salam:fix/reject-non-ascii-weekday-digits
Open

Reject non-ASCII digits in vWeekday ordwk (BYDAY/BYWEEKDAY/WKST)#1488
Labib-Bin-Salam wants to merge 1 commit into
collective:mainfrom
Labib-Bin-Salam:fix/reject-non-ascii-weekday-digits

Conversation

@Labib-Bin-Salam

@Labib-Bin-Salam Labib-Bin-Salam commented Jun 21, 2026

Copy link
Copy Markdown

Linked issue

No related issue; reporting directly (same as #1465).

Description

vWeekday silently accepts non-ASCII digits in the ordwk (relative) part of a BYDAY / BYWEEKDAY / WKST value.

Repro: vWeekday("١٢MO") (Arabic-Indic "12"), or parse a calendar containing RRULE:FREQ=MONTHLY;BYDAY=١٢MO.

>>> from icalendar import vWeekday
>>> w = vWeekday("١٢MO")   # Arabic-Indic digits
>>> w.relative
12

Cause: WEEKDAY_RULE matches the relative part with (?P<relative>[\d]{0,2}). In Python's default Unicode mode, \d is True for non-ASCII decimal digits (Arabic-Indic ١٢, extended-Arabic ۱۲, Devanagari, …). int() then parses those, so a malformed ordwk is silently accepted and normalized to a valid relative number. RFC 5545 §3.3.10 allows only ASCII DIGIT here (ordwk = 1*2DIGIT).

Fix: match [0-9] instead of \d so the grammar agrees with what int() accepts. After the fix, vWeekday("١٢MO") raises ValueError like any other malformed weekday, while every valid ASCII value (MO, 2MO, -1SU, +3WE, 53SU) is unchanged.

This is the direct sibling of the recently merged vMonth fix (#1465), which fixed the same non-ASCII-digit class in BYMONTH.

Checklist

  • I've added a change log entry to /news, following the instructions in Change log entry format.
  • I've added or updated tests if applicable.
  • I've run and ensured all tests pass locally by following Run tests.
  • I've added or edited documentation, both as docstrings to be rendered in the API documentation and narrative documentation, as necessary. (N/A — behaviour fix; added an explanatory code comment only.)

Additional information

  • Added regression test test_non_ascii_ordwk_digits_rejected (Arabic-Indic and extended-Arabic digits, single- and double-digit) next to the existing vWeekday tests; it fails before the change and passes after.
  • ruff check and ruff format --check are clean on the changed files; the targeted prop/ (887) and recurrence/RRULE (444) suites pass locally. (The unrelated test_timezone_identification / test_with_doctest failures reproduce identically on a clean checkout in this environment and are not affected by this change.)
  • AI usage disclosure (per the "Responsible AI use" policy): this change was drafted with the assistance of Claude Opus 4.8 (claude-opus-4-8), run via Claude Code — used to locate this sibling of the vMonth fix and to draft the regex change, the regression test, and the change log entry. All output was reviewed, executed, and verified by me, and I take responsibility for it. The model/version and usage are also recorded in the commit message and the disclosure is included in the change log entry.

📚 Documentation preview 📚: https://icalendar--1488.org.readthedocs.build/en/1488/

The WEEKDAY_RULE regular expression matched ``\d`` for the ``ordwk``
(relative) part. In Python's default Unicode mode ``\d`` is True for
non-ASCII digits such as the Arabic-Indic "12", and ``int`` then parses
them, so a value like "<Arabic-Indic 12>MO" was silently accepted with
relative == 12 instead of being rejected. RFC 5545, section 3.3.10 allows
only ASCII DIGIT here (ordwk = 1*2DIGIT). Match [0-9] so the grammar
agrees with what int() accepts. This mirrors the earlier vMonth non-ASCII
digit fix.

Added a regression test covering Arabic-Indic and extended-Arabic digits,
and a change log entry. Existing valid values (e.g. 2MO, -1SU, +3WE, 53SU)
are unaffected.

AI usage disclosure (per CONTRIBUTING, "Responsible AI use"):
- Model: Claude Opus 4.8 (claude-opus-4-8), run via Claude Code.
- How used: assisted in identifying this sibling of the merged vMonth fix,
  drafting the one-line regex change, the regression test, and the change
  log entry. All output was reviewed, executed, and verified by the author
  (targeted prop and recurrence suites plus ruff pass locally).
@github-actions

Copy link
Copy Markdown
Contributor

Profile summary:

GitHub user: Labib-Bin-Salam
🟢 No concerns found with user's profile.
🟡 Some concerns found with recent PR activity.
🟢 No concerns found with recent issue activity.

For a more detailed report, run `gh-profiler Labib-Bin-Salam`.
Full profile
GitHub user: Labib-Bin-Salam
🟢 No concerns found with user's profile.
   🟢 Account age: 4 years
   🟢 Profile information:
        name: Labib Bin Salam
        company: London South Bank University
        blog: https://www.datascienceportfol.io/LabibSalam#
        location: London
        bio:
          🚀 IT Consultant @TRL | Specializing in, Cloud-Native Infrastructure & Asset, Safety-Critical Data Systems.
          📍 London, UK | Python • Azure • SQL • Data Science
        linkedin: https://www.linkedin.com/in/labib-bin-salam-86363419a/
      Empty fields: email

🟡 Some concerns found with recent PR activity.
   46 PRs opened in the last 21 days.
      0 opened against repos the user owns.
      0 opened against repos in publicly associated orgs.
      46 opened against external repos.

   🟡 14 of 46 external PRs closed without merging in the last 21 days.

🟢 No concerns found with recent issue activity.
   🟢 No new issues opened in the last 21 days.

@read-the-docs-community

Copy link
Copy Markdown

Documentation build overview

📚 icalendar | 🛠️ Build #33235058 | 📁 Comparing de6a714 against latest (16fe2ab)

  🔍 Preview build  

4 files changed
± 404.html
± contribute/index.html
± _modules/icalendar/parser/string.html
± _modules/icalendar/prop/recur/weekday.html

r"""Non-ASCII digits must not be accepted in the ``ordwk`` (relative) part.

``\d`` is ``True`` for non-ASCII digits (e.g. Arabic-Indic ones), and
``int`` parses them, so an Arabic-Indic ``"12MO"`` was silently accepted

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"12MO" is the valid version, did you mean "١٢MO"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants