NUAAwqy opened a new pull request, #1770: URL: https://github.com/apache/casbin/pull/1770
## What A `#` or `;` inside a quoted value of a model `.conf` file is treated as the start of a comment, so the value is silently truncated and enforcement fails. ```ini [matchers] m = r.sub == p.sub && r.act == "#write" # only writing is allowed ``` The value `#write` contains a comment marker, so the matcher is read as: ``` r.sub == p.sub && r.act == " ``` and `Enforce` returns `unclosed string literal` instead of a decision. A matcher literal that legitimately contains `#` or `;` (a URL fragment, a tag, a role name) cannot be written in a model file at all today. ## Root cause Two places cut the line at the first `#`/`;` without knowing about quoted literals: 1. `config/config.go` — `parseBuffer()` scans the raw line for `DEFAULT_COMMENT` / `DEFAULT_COMMENT_SEM` and writes only the prefix before `write()` ever splits `option = value`, so the truncation happens while the file is being read. 2. `util/util.go` — `RemoveComments()` used `strings.Index(s, "#")`. It is applied to the matcher in `model/model.go`, `enforcer.go` and `management_api.go`, so even a value that survives parsing is truncated again on the way to the expression engine. `EscapeAssertion()` already skips quoted literals when rewriting `r.`/`p.` prefixes (added in 9a5c253), so the two comment scanners were the remaining inconsistent case. ## Fix Skip a terminated string literal (with `\` escaping) when looking for the comment marker, in both scanners: - `config/config.go`: the inline byte scan is replaced by `optionValueEnd()`, which returns the first `#`/`;` outside a quoted literal, or the end of the line. - `util/util.go`: `RemoveComments()` walks the string the same way. The literal scanner that `EscapeAssertion()` already needed is factored out into `stringLiteralEnd()` and reused; `EscapeAssertion()` behavior is unchanged. An **unterminated** literal is deliberately left alone: `m = r.act == "#write` still becomes `r.act == "`, so the expression parser reports the missing quote rather than the config parser hiding it behind a shortened value. ## Tests Added regression tests, all of which fail on current `master` and pass with the fix: - `TestCommentMarkerInsideQuotedValue` (`config/config_test.go`): `#`, single-quoted `#`, escaped-quote `#`, and the "value + real comment" combinations through the actual file/text parser. - `TestRemoveComments` (`util/util_test.go`): the same cases at the util level, plus `# it's a comment` (an apostrophe in a trailing comment must not swallow the rest of the line) and the unterminated-literal case. - `TestMatcherWithStringLiteralContainingHash` (`enforcer_test.go`): end-to-end `NewModelFromString` + `Enforce`, asserting `#write` is allowed and `read` is denied. Verification performed locally against `master` (d3b2659): - Before the fix: the three new tests fail (config values truncated to `r.act == "`, and `Enforce Error: unclosed string literal`). - After the fix: `go test ./...` passes for the whole module, including the existing `TestGet`/`TestEscapeAssertion`/`TestRemoveComments` cases, and `gofmt -l` is clean. ## Notes The `config` helper is a `[]byte` variant duplicated from the `util` one rather than shared: `config` currently depends only on the standard library, and reaching into `util` (which pulls in `rbac`) to reuse a 12-line scanner over a different type seemed like the worse trade. I'm happy to switch to a shared exported helper if you'd prefer that. This PR is deliberately limited to the comment/quoted-value interaction; no other parser behavior changes. Disclosure: I used an AI-assisted agent to help locate the two truncation sites, draft the fix, and write the tests; the reproduction and the verification results above are my own, and the code is what I would submit and maintain. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
