Comments (16)
Update: looking at the changes in the h file, the biggest change seems to be that HTTP codes have been added, on the c files however, there are many changes, 99% semantic, but 1% could be useful
Suggestion: instead of upgrading, only update the relevant parts.
Suggestion 2: instead of upgrading or updating, replace the out of maintenance http_parser library with this parser from Nginx, it's worth noting this is the parser that the nodeJS library is based on.
from crow.
@rijojosephc Yes I disregarded it when I saw that 72% was TypeScript. Let me know if i'm wrong and the 72% is just interface, with a C library similar to http-parser still being there.
In any case, migrating to any parser, be it Nginx or llhttp (without @ipkn's help) requires quite a large effort due to the fact that http_parser.h
has been modified quite a bit and is no longer directly compatible even with the original nodejs/http-parser.
from crow.
On the one side, it would be somewhat convenient to have a kind of all-in-one package, but Crow's main purpose is to provide a server and I'm not quite sure how often one need's to get another resource on the server-side. If one really needed that, he could still use cpr.
from crow.
ok so a bit of history:
- Crow was not always header only.
- Crow used to use
http_parser
as a submodule (back when the repo wasjoynet/http_parser
). - Crow became header only, at that time @ipkn merged
http_parser
into 1 header file. - Between 3 and 2017,
http_parser.h
was modified, moved into the include forlder, and some other stuff.
The clear change is adding PATCH and PURGE methods, simple enough, what's not simple is that XX
was renamed to CROW_XX
and there's no commit mentioning it.
2 things are required to get http_parser upgraded:
- Figure out how to merge the whole repo into 1 header file.
- get a diff between the 2 files and if a change seems to be made by @ipkn, apply it in the new file.
from crow.
I think second suggestion is the best.
from crow.
@The-EDev : Have we already considered the possibility of migrating to llhttp which claims to be the better maintained port of http-parser itself?
from crow.
@The-EDev In the README of llhttp I saw the usage with #include "llhttp.h"
.
I have not tried llhttp yet. Just wanted to check if you have already tried it.
If http_parser.h
is a heavily modified version of the original, then it will indeed be quite a bit of effort to "migrate".
from crow.
Going through the README, it seems that the parser is written in TypeScript then compiled into C. Something about that rubs me the wrong way.
I don't mind generating code that is tedious to write (see mime_types.h
). but the whole HTTP parser? I don't know if I like that idea..
from crow.
Looking at Nginx's Http parser, I can see a few ways of migrating crow to it.
The first is to take ngx_http_parse.c
and any dependencies it has, then slowly carve out any pieces not required for crow to operate and then changing all the ngx_
names, then use crow's parser to interface with nginx's parser and convert nginx's request to a crow request. This is the easier route made slightly more difficult by the lack of documentation on how @ipkn managed to do it with the original parser.
The second is to take ngx_http_parse.c
and replace any dependencies or parts to which a crow alternative exists with said crow alternative (e.g. replace ngx_http_request_t
with crow::request
and adapt nginx's parser to the change). This will in the short term downgrade crow's http-parser wrapper to just a simple interface and in the long term completely remove it.
A third option where the nginx extras are taken out of ngx_http_parse.c
and the remaining file changed to adapt to the existing http-parser wrapper is possible, and is a good balance between the first 2 options.
I personally prefer option 2, but am still open to discussion.
from crow.
Alright first update, The new parser is in a state of messiness, but I think I have the basics down. I'm assuming I'll need only 2 of the 10 functions included (parse request line and parse header) the rest are for 3 kinds of URIs (already neing done in router), Cookies (being done in middleware), arguments (being done in qs_parse), responses (not needed because crow's not a client), chunked requests (should investigate) and multi line headers (should also investigate).
The plan for now is to just get the thing working (primarily by replication nginx's data structures and later converting them to crow structures) and then move on to phasing out everything nginx and using crow's data structures.
from crow.
Testing for ngx_http_parse_request_line
(parses the very first line of a request) seems to be working flawlessly.
Moving on to test ngx_http_parse_header_line
.
UPDATE: ngx_http_parse_header_line
works flawlessly as well. The next step is to link the new parser to the wrapper.
from crow.
Day 3 of working on this:
The Parser wrapper is a shell of its former self, I expect it to be completely phased out by the time a PR is made, or by the next PR relating to the parser.
I was able to successfully create a crow::request
object with all the correct data (I've not tested parameters yet, but they should work too).
I'll keep the wrapper for now, just to keep the http_connection code undisturbed. And will try to get the code in a state where I can run unit tests.
UPDATE: Hello world example works, unit tests hang. next step is to get them working, and start trimming unnecessary parts
from crow.
I finally managed to get through the brick wall that is the unit tests (only the first problematic unit test actually), I had to revamp the way the parser works to support messages being sent in pieces, only thing left is parsing the body in a way where content-length
is taken into account (so that a request isn't processed until the full body is sent)
from crow.
The new parser is proving extremely difficult to set up, sure it works most of the time.. but there are multiple random issues that in all honesty make no sense to me. I might have to change my approach, cutting unnecessary parts from the existing parser while making sure everything works until the transition from old to new is as smooth as can be
from crow.
I went through the old parser and found a few pieces of code that could be taken out, most of them relating to response parsing. Taking these parts out will do 2 things:
- Set up the internal parts of the parser to be replaced with their Nginx counterparts.
- Make it extremely difficult to add HTTP client support to Crow (as in Crow making requests).
I'm torn on this subject, @mrozigor, @luca-schlecker What do you guys think? Are we ever going to add something like running Crow as a client? Or should I go ahead and modify the parser?
from crow.
Update:
I thought it would be best to upgrade the parser to v2.9.4
(the latest before the parser became unmaintained) before making any changes. And once that's done I can integrate it more with Crow and possibly either keep the modified version or swap it with another parser (most likely Nginx's).
So far I managed to go through about 40%-50% of the commits since the parser was integrated into Crow. I've reached version v2.6.0
, and skipped 6 commits (either for solving problems already solved in Crow or completely breaking compatibility with the wrapper).
Another Update:
I reached v2.7.1
(commit 1b79abab34d4763c0467f1173a406ad2817c1635
) with almost no trouble, This is significant (at least sentimentally) because that is the last commit that affects Crow which was published before it was abandoned in 2017, any new additions beyond this will be beyond the original repository's updates.
from crow.
Related Issues (20)
- Extend GitHub action to build with gcc and clang on the platforms
- Chat Example miss-leading
- Websocket Question HOT 4
- How can i know the progress of one post request? HOT 2
- Easily produced 'Worker Crash' with many simple concurrent http requests
- debian package is missing from the release section of version 1.1.0
- Please add new released version 1.1.0 to vcpkg HOT 1
- Compiler-Warning: ISO C++11 requires whitespace after the macro name
- How to use Middleware with CROW Blueprint HOT 13
- Websocket with dynamic routes in 1.1.0 HOT 1
- Update master to Conan 1.1.0 HOT 6
- Undocummented Macros HOT 2
- Mustache rendering adding extra quotes HOT 1
- Crow_enable_ssl HOT 1
- Crow Authorization Headers not working with CORs HOT 2
- Crow async usability HOT 2
- Troubles with crow running with apache2
- Cookie work cases HOT 1
- Websocket random generator - while loop HOT 4
- Error in installation HOT 4
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 crow.