django-easy-audit icon indicating copy to clipboard operation
django-easy-audit copied to clipboard

Logging requests with a URL > 254 charcaters raises an exception and fails the request

Open sycdan opened this issue 5 years ago • 5 comments

With request logging enabled, if I navigate to a very long URL (whether or not it's a valid URL) such that environ['PATH_INFO'] contains more than 254 characters, this exception is raised: django.db.utils.DataError: value too long for type character varying(254)

This has the side effect of causing the request to fail with status 500, even if the URL was valid.

Perhaps the RequestEvent.url field should be a TextField? Either that or the input should be truncated at 254 chars. Truncation seems sub-optimal as it could mean a loss of data, though. A combination of both might work, in addition to a setting for MAX_URL_LENGTH that defaults to 254 but can be increased if your application actually has really long URLs that you need to log requests for.

sycdan avatar Dec 11 '19 21:12 sycdan

The query params should be in a separate field already.

How could your URL itself be that long?

If we make it a text field we cannot index it.

Can you share some further detail/insight?

But yes I agree we should not crash. That sounds like a good bug fix.

jheld avatar Dec 11 '19 23:12 jheld

Yeah, it's just the path, not including the params. This actually came up because we had this mysterious unhandled exception occur after some bot (or so I assume) tried to hit our app with a really verbose GET request that looked as though it was trying to do some kind of SQL injection. Nothing bad happened, it was just an odd thing to find in the logs, when normally these junk requests would just return 404.

None of our actual URLs are anywhere near 254 characters, and I expect that's going to be true for most applications, so maybe a TextField isn't necessary. In that case, though, I guess the path would have to be truncated to 254 and maybe that should be noted as a caveat in the docs.

sycdan avatar Dec 12 '19 00:12 sycdan

Getting an error too from datatables.

/vms/feed/?sEcho=2&iColumns=11&sColumns=%2C%2C%2C%2C%2C%2C%2C%2C%2C%2C&iDisplayStart=0&iDisplayLength=10&mDataProp_0=0&sSearch_0=&bRegex_0=false&bSearchable_0=true&bSortable_0=true&mDataProp_1=1&sSearch_1=&bRegex_1=false&bSearchable_1=true&bSortable_1=true&mDataProp_2=2&sSearch_2=&bRegex_2=false&bSearchable_2=true&bSortable_2=true&mDataProp_3=3&sSearch_3=&bRegex_3=false&bSearchable_3=true&bSortable_3=true&mDataProp_4=4&sSearch_4=&bRegex_4=false&bSearchable_4=true&bSortable_4=true&mDataProp_5=5&sSearch_5=&bRegex_5=false&bSearchable_5=true&bSortable_5=true&mDataProp_6=6&sSearch_6=&bRegex_6=false&bSearchable_6=true&bSortable_6=true&mDataProp_7=7&sSearch_7=&bRegex_7=false&bSearchable_7=true&bSortable_7=true&mDataProp_8=8&sSearch_8=&bRegex_8=false&bSearchable_8=true&bSortable_8=true&mDataProp_9=9&sSearch_9=&bRegex_9=false&bSearchable_9=true&bSortable_9=true&mDataProp_10=10&sSearch_10=&bRegex_10=false&bSearchable_10=false&bSortable_10=false&sSearch=V0&bRegex=false&iSortingCols=0&_=1576119404515

andrewfam avatar Dec 12 '19 02:12 andrewfam

Looking through the code:

request_event = RequestEvent.objects.create(
        url=environ['PATH_INFO'],
        method=environ['REQUEST_METHOD'],
        query_string=environ['QUERY_STRING'],
        user_id=getattr(user, 'id', None),
        remote_ip=environ[REMOTE_ADDR_HEADER],
        datetime=timezone.now()
    )

We're just grabbing from the environ. And there is no max_length specified on the query_string on the model, so I don't know why the datatable URL would fail out. This may warrant further investigation, although yes I do agree that having error handling (and logging the error) will solve the initial issue, at least.

We're currently waiting for 1.1.2rc1 to go 1.1.2, so ideally we could get something merged for this by end of year (with a possible release end of year, too).

jheld avatar Dec 12 '19 13:12 jheld