Comments (9)
That also works. I was thinking more like:
auto add_preflight(const std::string& resource, const preflight_config& cfg) -> bool {
return add(http::method::options, resource, [cfg](const auto& req) { /* setup response based on cfg */ return res; });
}
That way the user has a preflight config that can be copied and tweaked across multiple instances and isn't tied to a single endpoint. Also we could even add logic for capture group usage if we wanted to.
Having it as its own endpoint might be cleaner in the long run though I guess? (though we would have to reimplement the regex matching of the regex endpoint if we wanted regex so I'm not sure)
from malloy.
Today we are really good at talking about the same thing but still "disagreeing" :p
from malloy.
I still haven't decided how to do this properly.
I was hoping that we could deduce the information from the controller's config somehow but that doesn't seem to be a very elegant solution given that the the Origin
header field might have different information. For example, it needs to include the schema (eg. http://
or https://
) and might also list a domain rather than just IP + port.
Currently I think that the best solution is to add a base_url
field of type std::string
to server::controller::config
. This needs to be passed to server::router
which would be a good time to create a server::router::config
struct which will also hold the shared_ptr
to the logger. Furthermore, the current m_generate_preflights
field might also be added to that struct.
It might be worth considering having the base_url
field in server::controller::config
be of type std::shared_ptr<const std::string>
instead to avoid unnecessary copies/duplicates. However, that would make filling the configuration struct less elegant.
One might instead go for adding std::enable_shared_from_this()
to controller::config
and pass an std::shared_ptr<const server::controller::config>
to the router
. Then again, this would give the router
access to fields such as the controller's logger which is... not elegant either.
@0x00002a Thoughts?
from malloy.
From my perspective I need to be able to have CORS access for webui backends and such. Off the top of my head, maybe it could also be done like the current routes, with an add_preflight
method that allowed generation of the preflight (essentially just add(...)
but with method::options
), and then m_generate_preflights
could be used to automatically do it in the absence of a dedicated handler.
Currently I have to disable the automatic preflight stuff and handle options
manually with my own response setup with the needed Access-Control-Allow-Origin
.
from malloy.
I think there might have been a slight misunderstanding. I am not proposing to drop this feature. I too need it for exactly the same reason :p
What I'd like to discuss is the design of how this should work - especially how the necessary information for filling the preflight response is passed into the router
.
I agree that preflights should only be generated automatically if none was provided manually.
from malloy.
Yeah sorry, thats what I meant too. What I mean is, I need to have a way to specify the Access-Control-Allow-Origin
and the current API doesn't provide it unless I disable part of it an do it by hand. So if the API is being improved I vote for allowing the user to specify any kind (so, what you said above :p).
I also have other custom setup for the preflight such as allowed headers, and methods. Speaking of which the methods should probably be filled automagically, since we have that information. Perhaps we could expose an object that allowed customising these things, exposing the set(http::field)
methods from beast::http::header
and providing QoL stuff like an "autogenerate allowed methods" switch or something. Then have a method in router
that attached it to specific endpoints via regex (which would also allow the user to make it as specific or general as they like).
Just some ideas
from malloy.
Improving the API is the entire point of this discussion/issue
So, how about creating struct preflight_config
. The router will have an instance of that which will be used to automatically generate preflights (if supposed to).
Speaking of which the methods should probably be filled automagically, since we have that information.
That is already the (somewhat?) the case:
malloy/lib/malloy/server/routing/router.cpp
Line 170 in 8d0529b
from malloy.
That is already the (somewhat?) the case:
So, how about creating struct preflight_config. The router will have an instance of that which will be used to automatically generate preflights (if supposed to).
I agree but I think it would be helpful to have a per-route config rather than a global or nothing situation, since certain parts of an API may have different requirements but still be mostly the same. If we allow the user to set it via regex for which routes it covers, it could reduce boilerplate/pain for the user while still being flexible enough to be global if the user wants or pinpoint precise as well. I was thinking we could actually just do this as a wrapper on router::add
, either wrapped in a lambda or give the config struct an operator()
.
Thoughts?
from malloy.
I agree but I think it would be helpful to have a per-route config rather than a global or nothing situation, since certain parts of an API may have different requirements but still be mostly the same.
Well, each (sub-)router would have its own instance of the preflight_config
:p
If we allow the user to set it via regex for which routes it covers, it could reduce boilerplate/pain for the user while still being flexible enough to be global if the user wants or pinpoint precise as well.
+1
I was thinking we could actually just do this as a wrapper on router::add, either wrapped in a lambda or give the config struct an operator().
Sounds like a good plan.
Just thinking about it - why not just adding another endpoint that is specialized for preflight requests? We could have something like:
struct preflight_config
{
std::string origin;
// ...
}
struct endpoint_http_preflight :
endpoint,
resource_matcher
{
preflight_config cfg;
};
and
bool router::add_preflight(/* ... */);
We can still have the router to optionally generate preflights automatically (if none was specified manually). router::set_generate_preflights()
needs to be adapted to accept a preflight_response
.
from malloy.
Related Issues (20)
- [Discussion] Why does `server::controller` use `shared_ptr` for the router, and why does `router` use `shared_ptr` for adding subrouters HOT 6
- TLS examples should load certs from memory HOT 4
- Release v0.4.0 HOT 1
- Application Framework demo broken
- Expose `run_result_t` publicly HOT 4
- Renaming `controller_run_result` HOT 9
- Usage of start() HOT 9
- Client: SNI hostname on WSS connection
- HTTP: More centralized configuration
- Connection timeout configuration
- Cross-Platform Way to Get Root Certs From Store? HOT 4
- server::listener: provide method for catching binding errors
- HTML form: check for requires fields HOT 2
- HTTP server: Add cache control HOT 1
- HTML form: populate() potentially dangerous HOT 1
- Custom HTTP response generator
- controller i/o threads reservation
- Form parsing should honor Content-Type
- Deprecate HTML namespace
- Improving access policy handling HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from malloy.