Skip to content

Commit

Permalink
[dio] Ensure consistent handling of "/" in concatenation of baseUrl w…
Browse files Browse the repository at this point in the history
…ith path (#1873)

This PR ensures consistent handling of "/" when concatenating the
baseUrl with the path.
Previously, if the baseUrl had a trailing "/" or the path had a leading
"/", the resulting URL would contain duplicate slashes.
To address this issue, the algorithm has been modified to insert only a
single "/" regardless of whether the baseUrl has a trailing slash or the
path has a leading slash. This improvement enhance the reliability and
correctness of the concatenation process, providing a more robust
solution.

### New Pull Request Checklist

- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [x] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package

---------

Signed-off-by: Jonas Uekötter <ueman@users.noreply.github.com>
Co-authored-by: Jonas Uekötter <ueman@users.noreply.github.com>
  • Loading branch information
KernelPanic92 and ueman authored Jun 30, 2023
1 parent 22bfe25 commit 1c640a6
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 9 deletions.
1 change: 1 addition & 0 deletions dio/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ See the [Migration Guide][] for the complete breaking changes list.**

## Unreleased

- Fix url concatenation '/' handling
- Remove `http` from `dev_dependencies`.

## 5.2.1+1
Expand Down
21 changes: 12 additions & 9 deletions dio/lib/src/options.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:io';

import 'package:meta/meta.dart';

import 'adapter.dart';
Expand Down Expand Up @@ -585,20 +587,21 @@ class RequestOptions extends _RequestConfig with OptionsMixin {

/// generate uri
Uri get uri {
String url = path;
if (!url.startsWith(RegExp(r'https?:'))) {
url = baseUrl + url;
final s = url.split(':/');
if (s.length == 2) {
url = '${s[0]}:/${s[1].replaceAll('//', '/')}';
}
Uri uri = Uri.parse(path);

if (!uri.isAbsolute) {
uri = Uri.parse(baseUrl).resolveUri(uri);
}

final query = Transformer.urlEncodeQueryMap(queryParameters, listFormat);
if (query.isNotEmpty) {
url += (url.contains('?') ? '&' : '?') + query;
final separator = uri.query.isNotEmpty ? '&' : '';
final mergedQuery = uri.query + separator + query;
uri = uri.replace(query: mergedQuery);
}

// Normalize the url.
return Uri.parse(url).normalizePath();
return uri.normalizePath();
}

/// Request data, can be any type.
Expand Down
84 changes: 84 additions & 0 deletions dio/test/options_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -469,4 +469,88 @@ void main() {
expect(response.data, 'test');
}
});

test('Ensure consistent slash handling', () {
final dio = Dio();
final inputs = [
['https://www.example.com', 'path'],
['https://www.example.com/', 'path'],
['https://www.example.com', '/path'],
['https://www.example.com/', '/path'],
];

for (final input in inputs) {
final baseUrl = input[0];
final path = input[1];
dio.options.baseUrl = baseUrl;
final actual = Options().compose(dio.options, path).uri.toString();
expect(actual, equals('https://www.example.com/path'));
}
});

test('Should return absolute URI when path is already absolute', () {
final baseUrl = 'https://www.example.com';
final path = 'https://www.another-example.com/path/to/resource';
final baseOptions = BaseOptions(baseUrl: baseUrl);

final actual = Options().compose(baseOptions, path).uri;

expect(actual.toString(), equals(path));
});

test('Should resolve relative path with base URL', () {
final baseUrl = 'https://www.example.com';
final path = '/path/to/resource';
final baseOptions = BaseOptions(baseUrl: baseUrl);

final actual = Options().compose(baseOptions, path).uri;

expect(
actual.toString(),
equals('https://www.example.com/path/to/resource'),
);
});

test('Should add query parameters to the URI', () {
final baseUrl = 'https://www.example.com';
final path = '/path/to/resource';
final baseOptions = BaseOptions(
baseUrl: baseUrl,
queryParameters: {'param1': 'value1'},
);

final actual = Options().compose(
baseOptions,
path,
queryParameters: {'param2': 'value2'},
).uri;

expect(
actual.queryParameters,
equals({
'param1': 'value1',
'param2': 'value2',
}),
);
});

test('Should preserve path query parameters to the URI', () {
final baseUrl = 'https://www.example.com';
final path = '/path/to/resource?param1=value1';
final baseOptions = BaseOptions(baseUrl: baseUrl);

final actual = Options(listFormat: ListFormat.csv).compose(
baseOptions,
path,
queryParameters: {'param2': 'value2'},
).uri;

expect(
actual.queryParameters,
equals({
'param1': 'value1',
'param2': 'value2',
}),
);
});
}

0 comments on commit 1c640a6

Please sign in to comment.