Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pathArgs method in analogy to the args method of ESP8266WebServer #9100

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ahorn42
Copy link

@ahorn42 ahorn42 commented Mar 10, 2024

If you want to use path arguments (pathArg) together with UriRegex and optional catch groups in a regular expression you need to know how many path arguments have been found as the number might vary. Otherwise you might access indices that are not available depending on the number of matched optional catch groups.

This pull request implements the method pathArgs in analogy to the method args for normal server arguments.

Feel free tell me if something should be changed or if something is missing and also feel free to do changes your self to this PR if you want to.

I hope this features helps someone else as well :) - it makes path matching with regular expressions even more powerful.

Andreas Horn added 2 commits March 10, 2024 12:26
This allows to use UriRegex with optional capture groups and therefore varying number of path arguments.
@mcspr
Copy link
Collaborator

mcspr commented Mar 17, 2024

(sry for delay)

I don't think Server params need to be renamed here?
Server method can keep the new name, but Request object can simply use a different method (or Server can friend it) name to access the internal object. Would be much shorter diff, too, and would not break backwards compat in case anyone used the old names

@ahorn42
Copy link
Author

ahorn42 commented Mar 17, 2024

Just to clarify - you mean just the parameter name pathArgs in the signature of the canHandle method?

bool canHandle(const String &requestUri, std::vector<String> &pathArgs) override final {

It that case I would say it might possibly lead to slight confusion, but in general I would be fine in changing that back :)

@mcspr
Copy link
Collaborator

mcspr commented Mar 17, 2024

I meant something like this

diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
index f7a95da0..9e6f299a 100644
--- a/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
+++ b/libraries/ESP8266WebServer/src/ESP8266WebServer-impl.h
@@ -581,6 +581,13 @@ void ESP8266WebServerTemplate<ServerType>::_streamFileCore(const size_t fileSize
   send(200, contentType, emptyString);
 }

+template <typename ServerType>
+int ESP8266WebServerTemplate<ServerType>::pathArgs() const {
+  if (_currentHandler != nullptr)
+    return _currentHandler->pathArgsSize();
+  return 0;
+}
+
 template <typename ServerType>
 const String& ESP8266WebServerTemplate<ServerType>::pathArg(unsigned int i) const {
   if (_currentHandler != nullptr)
diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer.h b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
index 397132f1..d7e6f9b5 100644
--- a/libraries/ESP8266WebServer/src/ESP8266WebServer.h
+++ b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
@@ -136,6 +136,7 @@ public:
   ServerType &getServer() { return _server; }

   const String& pathArg(unsigned int i) const; // get request path argument by number
+  int pathArgs() const;
   const String& arg(const String& name) const;    // get request argument value by name
   const String& arg(int i) const;          // get request argument value by number
   const String& argName(int i) const;      // get request argument name by number
diff --git a/libraries/ESP8266WebServer/src/detail/RequestHandler.h b/libraries/ESP8266WebServer/src/detail/RequestHandler.h
index 4195f0ff..aacbb8b1 100644
--- a/libraries/ESP8266WebServer/src/detail/RequestHandler.h
+++ b/libraries/ESP8266WebServer/src/detail/RequestHandler.h
@@ -27,10 +27,14 @@ protected:
     std::vector<String> pathArgs;

 public:
-    const String& pathArg(unsigned int i) {
+    const String& pathArg(unsigned int i) const {
         assert(i < pathArgs.size());
         return pathArgs[i];
     }
+
+    int pathArgsSize() const {
+        return pathArgs.size();
+    }
 };

 } // namespace

Since it is not shadowing anything while in the Server class context

edit: Also, check out CI logs down below. It fails when trying to build our example sketches

@ahorn42
Copy link
Author

ahorn42 commented Mar 17, 2024

Ah, okay - I think I now got what you mean - I will have look at it, maybe not today anymore, but I try to have a look in the next days!

mcspr added a commit to mcspr/esp8266-Arduino that referenced this pull request Jul 25, 2024
similar to ::args(), returns number of parsed path arguments
shortened esp8266#9100
mcspr added a commit to mcspr/esp8266-Arduino that referenced this pull request Jul 26, 2024
similar to ::args(), returns number of parsed path arguments
shortened esp8266#9100
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