You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "lhotari (via GitHub)" <gi...@apache.org> on 2025/07/18 20:43:28 UTC

[PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

lhotari opened a new pull request, #24537:
URL: https://github.com/apache/pulsar/pull/24537

   ### Motivation
   
   #22829 breaks backwards compatibility with more advanced regular expressions since the regular expression library was switched from JDK's default `java.util.regex.Pattern` to use [RE2/J](https://github.com/google/re2j).
   
   For example, negative lookaheads do not work by design in RE2/J. A common use case would be to subscribe to all topics excepts topics with certain prefixes. For example a pattern like `persistent://public/default/(?!(test2|test3|test4)-).*` which would subscribe to all other topics in `public/default` namespace that don't start with `test2-`, `test3-` or `test4-`.
   
   ```java
   Pattern someTopicsInNamespace = ;
   Consumer<byte[]> someTopicsConsumer = pulsarClient.newConsumer()
                   .topicsPattern(Pattern.compile("persistent://public/default/(?!(test2|test3|test4)-).*"))
                   .subscriptionName("subscription-1")
                   .subscribe();
   ```
   
   In Pulsar 4.0.x Java client or broker, this currently fails with an exception 
   ```
   org.apache.pulsar.shade.com.google.re2j.PatternSyntaxException: error parsing regexp: invalid or unsupported Perl syntax: `(?!\`
   ```
   
   ### Modifications
   
   - encapsulate topics pattern usage into a specific interface `TopicsPattern` which contains methods that are used in the client and on the broker for topics pattern consumers
   - on the broker side, add a configuration option `topicsPatternRegexImplementation` which allows to select the `TopicsPattern` implementation from these types:
     - `RE2J_WITH_JDK_FALLBACK` is the default. It uses the RE2J implementation and falls back to the JDK implementation for backwards compatibility reasons when the pattern compilation fails with the RE2/j library.
     - `RE2J` is more performant but does not support all regex features (e.g. negative lookaheads).
     - `JDK` uses the standard Java regex implementation which supports all features but can be slower.
     - Bad or malicious regex patterns requiring extensive backtracing could cause high resource usage with `RE2J_WITH_JDK_FALLBACK` or `JDK` implementations. 
     - Users concerned about regex resource usage DoS vulnerabilities in Pulsar can choose to use the `RE2J` implementation with the tradeoff of not being able to use patterns that RE2J doesn't support.
   - TBD
   
   ### Documentation
   
   <!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
   
   - [ ] `doc` <!-- Your PR contains doc changes. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2219678920


##########
pulsar-common/src/main/java/org/apache/pulsar/common/topics/JDKTopicsPattern.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.common.topics;
+
+import java.util.regex.Pattern;
+
+class JDKTopicsPattern implements TopicsPattern {
+    private final Pattern pattern;
+
+    public JDKTopicsPattern(String regex) {
+        this.pattern = Pattern.compile(regex);

Review Comment:
   ## Regular expression injection
   
   This regular expression is constructed from a [user-provided value](1).
   
   [Show more details](https://github.com/apache/pulsar/security/code-scanning/161)



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2219544883


##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyParserTest.java:
##########
@@ -110,7 +117,7 @@ public void testProducer() throws Exception {
         PulsarClient client = PulsarClient.builder().serviceUrl(proxyService.getServiceUrl())
                 .build();
         Producer<byte[]> producer =
-                client.newProducer(Schema.BYTES).topic("persistent://sample/test/local/producer-topic")
+                client.newProducer(Schema.BYTES).topic("persistent://sample/local/producer-topic")

Review Comment:
   I see `Topic name format V1 is explicitly prevented`. But will these tests fail without these changes?



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari merged PR #24537:
URL: https://github.com/apache/pulsar/pull/24537


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2219554282


##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyParserTest.java:
##########
@@ -110,7 +117,7 @@ public void testProducer() throws Exception {
         PulsarClient client = PulsarClient.builder().serviceUrl(proxyService.getServiceUrl())
                 .build();
         Producer<byte[]> producer =
-                client.newProducer(Schema.BYTES).topic("persistent://sample/test/local/producer-topic")
+                client.newProducer(Schema.BYTES).topic("persistent://sample/local/producer-topic")

Review Comment:
   I'll check later. Testing for V1 wouldn't make much sense since it's been deprecated since Pulsar 2.0.0. However I agree that the change should be made separately to switch from V1 to V2 topic names in tests. We should also have an option to globally block V1 topics in a broker.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#issuecomment-3097556006

   ## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/24537?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `82.05128%` with `14 lines` in your changes missing coverage. Please review.
   > Project coverage is 74.29%. Comparing base [(`bbc6224`)](https://app.codecov.io/gh/apache/pulsar/commit/bbc62245c5ddba1de4b1e7cee4ab49334bc36277?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`9c232b7`)](https://app.codecov.io/gh/apache/pulsar/commit/9c232b75c549533fe061fd9d044f1e0caa247fb6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   > Report is 1208 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/24537?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...apache/pulsar/broker/service/TopicListService.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2Fservice%2FTopicListService.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1RvcGljTGlzdFNlcnZpY2UuamF2YQ==) | 42.85% | [7 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...che/pulsar/common/topics/TopicsPatternFactory.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Ftopics%2FTopicsPatternFactory.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvVG9waWNzUGF0dGVybkZhY3RvcnkuamF2YQ==) | 81.81% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...va/org/apache/pulsar/broker/service/ServerCnx.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2Fservice%2FServerCnx.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1NlcnZlckNueC5qYXZh) | 91.66% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [...he/pulsar/common/topics/MatchAllTopicsPattern.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Ftopics%2FMatchAllTopicsPattern.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvTWF0Y2hBbGxUb3BpY3NQYXR0ZXJuLmphdmE=) | 66.66% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   [![Impacted file tree graph](https://app.codecov.io/gh/apache/pulsar/pull/24537/graphs/tree.svg?width=650&height=150&src=pr&token=acYqCpsK9J&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #24537      +/-   ##
   ============================================
   + Coverage     73.57%   74.29%   +0.72%     
   - Complexity    32624    32944     +320     
   ============================================
     Files          1877     1874       -3     
     Lines        139502   146151    +6649     
     Branches      15299    16766    +1467     
   ============================================
   + Hits         102638   108587    +5949     
   - Misses        28908    28951      +43     
   - Partials       7956     8613     +657     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pulsar/pull/24537/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [inttests](https://app.codecov.io/gh/apache/pulsar/pull/24537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.76% <7.69%> (+2.18%)` | :arrow_up: |
   | [systests](https://app.codecov.io/gh/apache/pulsar/pull/24537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `23.37% <35.89%> (-0.96%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pulsar/pull/24537/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.79% <82.05%> (+0.94%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/24537?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...org/apache/pulsar/broker/ServiceConfiguration.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-broker-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2FServiceConfiguration.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvU2VydmljZUNvbmZpZ3VyYXRpb24uamF2YQ==) | `98.17% <100.00%> (-1.22%)` | :arrow_down: |
   | [...ar/client/impl/PatternMultiTopicsConsumerImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FPatternMultiTopicsConsumerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1BhdHRlcm5NdWx0aVRvcGljc0NvbnN1bWVySW1wbC5qYXZh) | `75.67% <100.00%> (-7.79%)` | :arrow_down: |
   | [...rg/apache/pulsar/client/impl/PulsarClientImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FPulsarClientImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1B1bHNhckNsaWVudEltcGwuamF2YQ==) | `75.12% <100.00%> (+0.81%)` | :arrow_up: |
   | [...rg/apache/pulsar/client/impl/TopicListWatcher.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-client%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fimpl%2FTopicListWatcher.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1RvcGljTGlzdFdhdGNoZXIuamF2YQ==) | `67.88% <100.00%> (+0.02%)` | :arrow_up: |
   | [...g/apache/pulsar/common/lookup/GetTopicsResult.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Flookup%2FGetTopicsResult.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi9sb29rdXAvR2V0VG9waWNzUmVzdWx0LmphdmE=) | `94.28% <ø> (+31.78%)` | :arrow_up: |
   | [.../apache/pulsar/common/topics/JDKTopicsPattern.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Ftopics%2FJDKTopicsPattern.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvSkRLVG9waWNzUGF0dGVybi5qYXZh) | `100.00% <100.00%> (ø)` | |
   | [...apache/pulsar/common/topics/RE2JTopicsPattern.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Ftopics%2FRE2JTopicsPattern.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvUkUySlRvcGljc1BhdHRlcm4uamF2YQ==) | `100.00% <100.00%> (ø)` | |
   | [...ava/org/apache/pulsar/common/topics/TopicList.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Ftopics%2FTopicList.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvVG9waWNMaXN0LmphdmE=) | `100.00% <100.00%> (+10.34%)` | :arrow_up: |
   | [...org/apache/pulsar/common/topics/TopicsPattern.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fcommon%2Ftopics%2FTopicsPattern.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NvbW1vbi90b3BpY3MvVG9waWNzUGF0dGVybi5qYXZh) | `100.00% <100.00%> (ø)` | |
   | [...va/org/apache/pulsar/broker/service/ServerCnx.java](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree&filepath=pulsar-broker%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fbroker%2Fservice%2FServerCnx.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1NlcnZlckNueC5qYXZh) | `72.48% <91.66%> (+0.33%)` | :arrow_up: |
   | ... and [3 more](https://app.codecov.io/gh/apache/pulsar/pull/24537?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
   
   ... and [1099 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/24537/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   </details>
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2222266557


##########
pulsar-common/src/main/java/org/apache/pulsar/common/topics/JDKTopicsPattern.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.common.topics;
+
+import java.util.regex.Pattern;
+
+class JDKTopicsPattern implements TopicsPattern {
+    private final String inputPattern;
+    private final Pattern pattern;
+
+    public JDKTopicsPattern(String inputPattern, String regexWithoutTopicDomainScheme) {
+        this.inputPattern = inputPattern;
+        this.pattern = Pattern.compile(regexWithoutTopicDomainScheme);

Review Comment:
   ## Regular expression injection
   
   This regular expression is constructed from a [user-provided value](1).
   
   [Show more details](https://github.com/apache/pulsar/security/code-scanning/162)



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2219542184


##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyParserTest.java:
##########
@@ -110,7 +117,7 @@ public void testProducer() throws Exception {
         PulsarClient client = PulsarClient.builder().serviceUrl(proxyService.getServiceUrl())
                 .build();
         Producer<byte[]> producer =
-                client.newProducer(Schema.BYTES).topic("persistent://sample/test/local/producer-topic")
+                client.newProducer(Schema.BYTES).topic("persistent://sample/local/producer-topic")

Review Comment:
   Why did you modify many namespaces from V1 to V2 in tests? (`sample/test/local` to `sample/local`) It seems irrelevant with the core changes.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2221406138


##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyParserTest.java:
##########
@@ -110,7 +117,7 @@ public void testProducer() throws Exception {
         PulsarClient client = PulsarClient.builder().serviceUrl(proxyService.getServiceUrl())
                 .build();
         Producer<byte[]> producer =
-                client.newProducer(Schema.BYTES).topic("persistent://sample/test/local/producer-topic")
+                client.newProducer(Schema.BYTES).topic("persistent://sample/local/producer-topic")

Review Comment:
   I'll push a force push where these topic name V1 related changes are reverted.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2221401957


##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyParserTest.java:
##########
@@ -110,7 +117,7 @@ public void testProducer() throws Exception {
         PulsarClient client = PulsarClient.builder().serviceUrl(proxyService.getServiceUrl())
                 .build();
         Producer<byte[]> producer =
-                client.newProducer(Schema.BYTES).topic("persistent://sample/test/local/producer-topic")
+                client.newProducer(Schema.BYTES).topic("persistent://sample/local/producer-topic")

Review Comment:
   @BewareMyPower I found the issue in supporting V1 topic names. I'll restore the support in this PR. 
   We really need to remove V1 topics from Pulsar at some point since it's a mess to keep on supporting them, but that would be taken care on separately.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2221692821


##########
pulsar-common/src/main/java/org/apache/pulsar/common/topics/JDKTopicsPattern.java:
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.common.topics;
+
+import java.util.regex.Pattern;
+
+class JDKTopicsPattern implements TopicsPattern {
+    private final Pattern pattern;
+
+    public JDKTopicsPattern(String regex) {
+        this.pattern = Pattern.compile(regex);

Review Comment:
   It's possible to configure the broker to use RE2J when a user is concerned about ReDoS vulnerabilities.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2222519641


##########
pulsar-common/src/main/java/org/apache/pulsar/common/topics/JDKTopicsPattern.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.common.topics;
+
+import java.util.regex.Pattern;
+
+class JDKTopicsPattern implements TopicsPattern {
+    private final String inputPattern;
+    private final Pattern pattern;
+
+    public JDKTopicsPattern(String inputPattern, String regexWithoutTopicDomainScheme) {
+        this.inputPattern = inputPattern;
+        this.pattern = Pattern.compile(regexWithoutTopicDomainScheme);

Review Comment:
   It's possible to configure the broker to use RE2J when a user is concerned about ReDoS vulnerabilities.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [fix][misc] Fix topics pattern consumer backwards compatibility [pulsar]

Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #24537:
URL: https://github.com/apache/pulsar/pull/24537#discussion_r2220752302


##########
pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyParserTest.java:
##########
@@ -110,7 +117,7 @@ public void testProducer() throws Exception {
         PulsarClient client = PulsarClient.builder().serviceUrl(proxyService.getServiceUrl())
                 .build();
         Producer<byte[]> producer =
-                client.newProducer(Schema.BYTES).topic("persistent://sample/test/local/producer-topic")
+                client.newProducer(Schema.BYTES).topic("persistent://sample/local/producer-topic")

Review Comment:
   Yes, I agree. 



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org