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

Fix memory allocation #380

Closed
humphd opened this issue Nov 25, 2021 · 9 comments
Closed

Fix memory allocation #380

humphd opened this issue Nov 25, 2021 · 9 comments
Labels
bug Something isn't working
Milestone

Comments

@humphd
Copy link

humphd commented Nov 25, 2021

I have a .hurl file that looks like this:

# 1. Start by creating a fragment for this user
POST http://localhost:8080/v1/fragments
Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==
Content-Type: text/markdown; charset=utf-8
```# This is a heading```

HTTP/1.1 201
[Captures]
url: header "Location"


# 2. Try to GET the fragment we just posted as HTML vs. Markdown
GET {{url}}.html
Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==

HTTP/1.1 200
Content-Type: text/html; charset=utf-8
```<h1>This is a fragment</h1>
```

When I run it with hurl, I get this:

hurl -v --test ./tests/integration/get-fragments-id-markdown-as-html.hurl
./tests/integration/get-fragments-id-markdown-as-html.hurl: RUNNING [1/1]
* fail fast: true
* insecure: false
* follow redirect: false
* max redirect: 50
* ------------------------------------------------------------------------------
* executing entry 1
*
* Cookie store:
*
* Request
* POST http://localhost:8080/v1/fragments
* Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==
* Content-Type: text/markdown; charset=utf-8
*
* request can be run with the following curl command:
* curl 'http://localhost:8080/v1/fragments' -H 'Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==' -H 'Content-Type: text/markdown; charset=utf-8' --data '# This is a heading'
*
> POST /v1/fragments HTTP/1.1
> Host: localhost:8080
> Accept: */*
> Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==
> Content-Type: text/markdown; charset=utf-8
> User-Agent: hurl/1.4.0
> Content-Length: 19
>
< HTTP/1.1 201 Created
< Content-Security-Policy: default-src 'self';base-uri 'self';block-all-mixed-content;font-src 'self' https: data:;frame-ancestors 'self';img-src 'self' data:;object-src 'none';script-src 'self';script-src-attr 'none';style-src 'self' https: 'unsafe-inline';upgrade-insecure-requests
< X-DNS-Prefetch-Control: off
< Expect-CT: max-age=0
< X-Frame-Options: SAMEORIGIN
< Strict-Transport-Security: max-age=15552000; includeSubDomains
< X-Download-Options: noopen
< X-Content-Type-Options: nosniff
< X-Permitted-Cross-Domain-Policies: none
< Referrer-Policy: no-referrer
< X-XSS-Protection: 0
< Access-Control-Allow-Origin: *
< Location: http://localhost:8080/v1/fragments/Yn7vDut1ipp0ToK7dtzCD
< Content-Type: application/json; charset=utf-8
< Content-Length: 185
< ETag: W/"b9-VSn3OImkcGrGMazPe+eoVKXtQ/M"
< Vary: Accept-Encoding
< Date: Thu, 25 Nov 2021 18:59:17 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
<
* Captures
* url: http://localhost:8080/v1/fragments/Yn7vDut1ipp0ToK7dtzCD
*
* ------------------------------------------------------------------------------
* executing entry 2
*
* Cookie store:
*
* Request
* GET http://localhost:8080/v1/fragments/Yn7vDut1ipp0ToK7dtzCD.html
* Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==
*
* request can be run with the following curl command:
* curl 'http://localhost:8080/v1/fragments/Yn7vDut1ipp0ToK7dtzCD.html' -H 'Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ=='
*
> GET /v1/fragments/Yn7vDut1ipp0ToK7dtzCD.html HTTP/1.1
> Host: localhost:8080
> Accept: */*
> Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==
> User-Agent: hurl/1.4.0
>
< HTTP/1.1 200 OK
< Content-Security-Policy: default-src 'self';base-uri 'self';block-all-mixed-content;font-src 'self' https: data:;frame-ancestors 'self';img-src 'self' data:;object-src 'none';script-src 'self';script-src-attr 'none';style-src 'self' https: 'unsafe-inline';upgrade-insecure-requests
< X-DNS-Prefetch-Control: off
< Expect-CT: max-age=0
< X-Frame-Options: SAMEORIGIN
< Strict-Transport-Security: max-age=15552000; includeSubDomains
< X-Download-Options: noopen
< X-Content-Type-Options: nosniff
< X-Permitted-Cross-Domain-Policies: none
< Referrer-Policy: no-referrer
< X-XSS-Protection: 0
< Access-Control-Allow-Origin: *
< Content-Type: text/html; charset=utf-8
< Content-Length: 27
< ETag: W/"1b-3RJCZQ+msy33EIcblC+yHS1FS2A"
< Vary: Accept-Encoding
< Date: Thu, 25 Nov 2021 18:59:17 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
<
*
memory allocation of 18446744073709551613 bytes failed
Abort trap: 6

Running it in curl works:

curl --include 'http://localhost:8080/v1/fragments/z6KrOksNIeLSZ3OIeq7or.html' -H 'Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ=='
HTTP/1.1 200 OK
Content-Security-Policy: default-src 'self';base-uri 'self';block-all-mixed-content;font-src 'self' https: data:;frame-ancestors 'self';img-src 'self' data:;object-src 'none';script-src 'self';script-src-attr 'none';style-src 'self' https: 'unsafe-inline';upgrade-insecure-requests
X-DNS-Prefetch-Control: off
Expect-CT: max-age=0
X-Frame-Options: SAMEORIGIN
Strict-Transport-Security: max-age=15552000; includeSubDomains
X-Download-Options: noopen
X-Content-Type-Options: nosniff
X-Permitted-Cross-Domain-Policies: none
Referrer-Policy: no-referrer
X-XSS-Protection: 0
Access-Control-Allow-Origin: *
Content-Type: text/html; charset=utf-8
Content-Length: 27
ETag: W/"1b-3RJCZQ+msy33EIcblC+yHS1FS2A"
Vary: Accept-Encoding
Date: Thu, 25 Nov 2021 18:58:48 GMT
Connection: keep-alive
Keep-Alive: timeout=5

<h1>This is a heading</h1>

@fabricereix
Copy link
Collaborator

Thanks for reporting, this is our first "memory allocation" bug.

We are going to try reproduce it with the same HTTP response.
Do you also have this error when you are not in verbose mode?

Thanks

@humphd
Copy link
Author

humphd commented Nov 25, 2021

Yes, it happens without verbose as well. Is there any other testing/data I can do here to share with you to help with debugging this?

@jcamiel
Copy link
Collaborator

jcamiel commented Nov 25, 2021

18446744073709551613 is 2**64 - 3. Probably the size of some object is (erroneously) computed as -3 byte!

The second response seems to trigger this bug, would it be possible maybe to « capture » the second HTTP response with a proxy (https://hurl.dev/docs/tutorial/debug-tips.html#using-a-proxy)
?

Or, if it is possible of course, could we reproduce your local server setup ?

@jcamiel
Copy link
Collaborator

jcamiel commented Nov 25, 2021

@humphd @fabricereix Ok I can reproduce it with a simple setup:

Flask file:

from tests import app
from flask import Response

@app.route("/issue380")
def issue380():
    headers = {
        'Content-Type': 'text/html; charset=utf-8',
    }
    return Response('<h1>This is a fragment</h1>', headers=headers)

Hurl file:

GET http://localhost:8000/issue380

HTTP/1.0 200
Content-Type: text/html; charset=utf-8
```<h1>This is a fragment</h1>
```

Execution:

$ hurl tests/issue380.hurl
memory allocation of 18446744073709551613 bytes failed
Abort trap: 6

@humphd, we have all we need to find and fix this bug, it's not worth capturing the response or trying to reproduce your setup. Now, I'm really curious about it, it should not be too difficult to isolate it now we can reproduce it easily

@humphd
Copy link
Author

humphd commented Nov 25, 2021

Great, I'm glad you can reproduce. I'll be interested to see what you discover as you debug.

@jcamiel
Copy link
Collaborator

jcamiel commented Nov 25, 2021

The bug is in the Hurl raw string body, when the raw string body is ending with a new line and we have to display an error:

let width = (error.source_info().end.column - error.source_info().start.column) as usize;

In this case, error.source_info().end.column is 1 and error.source_info().start.column is 4 and we cast 1-4 to an unsigned integer... A few line before, we have added this comment // TODO: to clean/Refacto, so we've been guilty of not cleaning this code sooner !! We should see why rust let us write this code without emitting some warnings.

Anyway, as a patch, you could write your Hurl file like this:

Before:

# ...

# 2. Try to GET the fragment we just posted as HTML vs. Markdown
GET {{url}}.html
Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==

HTTP/1.1 200
Content-Type: text/html; charset=utf-8
```<h1>This is a fragment</h1>
```

Patched:

# ...

# 2. Try to GET the fragment we just posted as HTML vs. Markdown
GET {{url}}.html
Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==

HTTP/1.1 200
Content-Type: text/html; charset=utf-8
[Asserts]
body == "<h1>This is a fragment</h1>\n"

Less clean, but this patch the bug in the raw string body.

Just a note:

```<h1>This is a fragment</h1>
```

is equivalent to the string "<h1>This is a fragment</h1>\n" (with a new line at the end) so you
expect to have a response body ended with a new line, whereas:

```<h1>This is a fragment</h1>```

is equivalent to "<h1>This is a fragment</h1>" (without a new line)
https://hurl.dev/docs/asserting-response.html#raw-string-body

Thank you for taking the time to write the issue, we're going to fix it asap on master

@jcamiel jcamiel added the bug Something isn't working label Nov 25, 2021
@humphd
Copy link
Author

humphd commented Nov 25, 2021

Excellent. Your body == "<h1>This is a fragment</h1>\n" assertion is working for me with hurl 1.4.0, so I'm able to continue while you fix. Glad I could help contribute something to hurl, I'm really enjoying using it.

@fabricereix fabricereix added this to the 1.5.0 milestone Nov 26, 2021
@fabricereix
Copy link
Collaborator

fabricereix commented Nov 26, 2021

Note than relating to the whitespace in the asserts,
we also plan to ignore them with semantic comparisons for JSON/XML (#270/#272) avoiding the use of raw string.

Your Hurl file will become

GET {{url}}.html
Authorization: Basic dXNlcjFAZW1haWwuY29tOnBhc3N3b3JkMQ==

HTTP/1.1 200
Content-Type: text/html; charset=utf-8
<h1>This is a fragment</h1>

@fabricereix
Copy link
Collaborator

The fix for the memory allocation has been merged into master (#382) and will be included in the 1.5.0.

But we still have to work on the assert error messages for raw strings, which are not clear with newline content.

@fabricereix fabricereix changed the title memory allocation of 18446744073709551613 bytes failed Abort trap: 6 Fix memory allocation Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants