Skip to content

Conversation

@derpflanz
Copy link

Implement OPTIONS method.

When using REST calls using a browser, the OPTIONS call is made as preflight, before any other calls are done.

void ArduinoHttpServer::AbstractStreamHttpReply::addHeader(const String& name, const String &value) {
if (m_headerCount == MAX_HEADERS) return;

m_headerNames[m_headerCount] = name;
Copy link
Owner

Choose a reason for hiding this comment

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

Why not a struct with a name and value pair?

// First strtok call, initialize with cached line buffer.
// len("DELETE") + 1 for terminating null = 7
FixString<7U> token(strtok_r(lineBuffer, " ", &m_lineBufferStrTokContext));
// len("OPTIONS") + 1 for terminating null = 7
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
// len("OPTIONS") + 1 for terminating null = 7
// len("OPTIONS") + 1 for terminating null = 8


#include "ArduinoHttpServerDebug.h"

#define MAX_HEADERS 3
Copy link
Owner

Choose a reason for hiding this comment

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

Can you change it to modern C++ typesafe constexpr?

Copy link
Author

Choose a reason for hiding this comment

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

I can give it a shot, but I haven't used those ever (I am not a native C++ speaker).


private:

String m_headerNames[MAX_HEADERS];
Copy link
Owner

Choose a reason for hiding this comment

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

If you don't mind, I think I will rewrite it to become an array of HttpField objects.

Copy link
Author

Choose a reason for hiding this comment

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

Of course I don't mind. Your C++ is probably better than mine :).

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