CodingAleCR / http_interceptor

A lightweight, simple plugin that allows you to intercept request and response objects and modify them if desired.
MIT License
133 stars 67 forks source link

Body is set to empty string when it should be null #121

Closed s-yanev closed 1 year ago

s-yanev commented 1 year ago

Describe the bug Currently when a GET request is invoked and the Request object is converted to RequestData in the RequestData.fromHttpRequest() function, even though the BODY in the Request object is null we end up with an empty string as body in the RequestData object.

This is wrong since later when the RequestData is converted back to Request object the empty string body is used and when the body is set as a String then automatically the Content-Type is set as text/plain in the headers, which is not acceptable for some APIs and leads to a 415 - Unsupported media type error. This can be checked in the http-0.13.5 package in the request.dart file in the body setter.

Expected behavior BODY should be kept as null.

Version:

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

s-yanev commented 1 year ago

Any thoughts on this? It would be nice to know if this was a desired effect or if it is a bug? If we agree this is actually a bug, I will be glad to provide a fix my self.

CodingAleCR commented 1 year ago

Not sure how I missed this. It is definitely a bug in v1.0.2. It is fixed in beta versions for the rewrite in 2.x but no in the stable version.

s-yanev commented 1 year ago

@CodingAleCR Just tested version 2.0.0-beta.6 and it seems the problem is still present. The issue now comes from the RequestCopyWith extension on line 24 :

Request copyWith({
    HttpMethod? method,
    Uri? url,
    Map<String, String>? headers,
    String? body,
    List<int>? bodyBytes,
    Encoding? encoding,
    bool? followRedirects,
    int? maxRedirects,
    bool? persistentConnection,
  }) {
    final copied = Request(
      method?.asString ?? this.method,
      url ?? this.url,
    )..body = this.body; // <---- here is the issue

    if (body != null) {
      copied.body = body;
    }

    if (bodyBytes != null) {
      copied.bodyBytes = bodyBytes;
    }

    return copied
      ..headers.addAll(headers ?? this.headers)
      ..encoding = encoding ?? this.encoding
      ..followRedirects = followRedirects ?? this.followRedirects
      ..maxRedirects = maxRedirects ?? this.maxRedirects
      ..persistentConnection =
          persistentConnection ?? this.persistentConnection;
  } 

I think if this line is removed and you leave for the next if statement to handle it everything should be working as expected.

I can also create a PR with the fix if you don't have the time to do it.

KwabenBerko commented 2 months ago

@s-yanev @CodingAleCR This issue still exists even in the 2.0.0 version. The content-type: text/plain; charset=utf-8 header seems to be automatically added toGET requests which ideally should not be the case.