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

feat(services/webdav): support redirection when get 302/307 response during read operation #2256

Merged
merged 44 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
b8a2c82
fix: add redirect logic
Gnosnay May 1, 2023
f6d5f4f
refactor: webdav redirect logic refactor
Gnosnay May 8, 2023
1e951d2
fix: compare Origin instead of whole url
Gnosnay May 11, 2023
3da6269
fix: path beginning with /
Gnosnay May 11, 2023
6600257
fix: remove the root from redirect location
Gnosnay May 11, 2023
9107919
fix: add url escape decode logic for path
Gnosnay May 11, 2023
fe10893
chores: format
Gnosnay May 11, 2023
e14820d
chores: add integration test for redirect
Gnosnay May 11, 2023
b77233f
fix: nginx config log file for new virtual server
Gnosnay May 12, 2023
12b213f
fix: use loopback nic to listen on
Gnosnay May 12, 2023
182e7a1
fix: proxy forward address
Gnosnay May 12, 2023
f00018a
fix: add module of webdav of nginx
Gnosnay May 12, 2023
2ff9a36
fix: permission error during test
Gnosnay May 12, 2023
cf03cee
fix: permission error during test
Gnosnay May 12, 2023
60e1579
tmp: add log for nginx
Gnosnay May 12, 2023
39229fc
tmp: add log for nginx
Gnosnay May 12, 2023
d2e7812
tmp: add log for nginx
Gnosnay May 12, 2023
154e281
tmp: add log for nginx
Gnosnay May 12, 2023
c71abc1
tmp: add log for nginx
Gnosnay May 12, 2023
54ab48d
tmp: add log for nginx
Gnosnay May 12, 2023
96e3447
tmp: add log for nginx
Gnosnay May 13, 2023
e4c1c80
tmp: add log for nginx
Gnosnay May 13, 2023
b3bb57d
tmp: add log for nginx
Gnosnay May 13, 2023
24cb37c
tmp: add log for nginx
Gnosnay May 13, 2023
6c303a3
fix: permission
Gnosnay May 13, 2023
f9fa903
debug: check permission staff
Gnosnay May 14, 2023
6507ed6
debug: make /var/lib/nginx/body executable
Gnosnay May 14, 2023
17486f6
fix: nginx redriect test
Gnosnay May 14, 2023
f369baf
refactor: refactor with maximum retry times
Gnosnay May 14, 2023
c465a45
refactor: remove async recursive dep
Gnosnay May 14, 2023
ecdf0fe
chore: clippy fix
Gnosnay May 14, 2023
ce7f9cf
chore: cargo fmt fix
Gnosnay May 14, 2023
cfac25b
fix: retry step fix
Gnosnay May 14, 2023
2f2a96b
fix: remove useless dep
Gnosnay May 14, 2023
6687fc1
fix: clippy fix and typos
Gnosnay May 14, 2023
4cd0335
fix: comment fixes
Gnosnay May 15, 2023
0009b96
chores: comment style change
Gnosnay May 18, 2023
6fb6e1d
refactor: revert redirect logic in webdav
Gnosnay May 22, 2023
e1c7e50
refactor: add redirect handling for http client
Gnosnay May 28, 2023
7638529
fix: cargo clippy
Gnosnay May 28, 2023
e861443
chore: fix format
Gnosnay May 28, 2023
1da0856
Merge branch 'main' of github.com:apache/incubator-opendal into featu…
Gnosnay May 28, 2023
60188f4
Merge branch 'main' of github.com:apache/incubator-opendal into featu…
Gnosnay May 30, 2023
8cf232e
Merge branch 'main' into feature/support-redirect-for-webdav
suyanhanx May 30, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions .github/workflows/service_test_webdav.yml
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,38 @@ jobs:
OPENDAL_WEBDAV_ENDPOINT: http://127.0.0.1:8080
OPENDAL_WEBDAV_USERNAME: bar
OPENDAL_WEBDAV_PASSWORD: bar

nginx_with_redirect:
runs-on: ${{ matrix.os }}
strategy:
matrix:
os:
- ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Setup Rust toolchain
uses: ./.github/actions/setup

- name: Install nginx full for dav_ext modules
run: sudo apt install nginx-full

- name: Start nginx
shell: bash
working-directory: core
run: |
mkdir -p /tmp/static
mkdir -p /var/lib/nginx
chmod a+rw /tmp/static/ # make nginx worker has permission to operate it
Xuanwo marked this conversation as resolved.
Show resolved Hide resolved
sudo chmod 777 /var/lib/nginx/body # make nginx worker has permission to operate it
nginx -c `pwd`/src/services/webdav/fixtures/nginx.conf

- name: Test with redirect
shell: bash
working-directory: core
run: |
cargo test webdav -- --show-output
env:
RUST_BACKTRACE: full
RUST_LOG: debug
OPENDAL_WEBDAV_TEST: on
OPENDAL_WEBDAV_ENDPOINT: http://127.0.0.1:8081
102 changes: 88 additions & 14 deletions core/src/services/webdav/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.

use std::borrow::Cow;
use std::collections::HashMap;
use std::fmt::Debug;
use std::fmt::Formatter;
Expand All @@ -27,6 +28,8 @@ use http::Request;
use http::Response;
use http::StatusCode;
use log::debug;
use percent_encoding::percent_decode_str;
use reqwest::Url;

use super::error::parse_error;
use super::list_response::Multistatus;
Expand Down Expand Up @@ -199,7 +202,7 @@ impl Builder for WebdavBuilder {
Some(v) => v,
None => {
return Err(Error::new(ErrorKind::ConfigInvalid, "endpoint is empty")
.with_context("service", Scheme::Webdav))
.with_context("service", Scheme::Webdav));
}
};

Expand Down Expand Up @@ -299,17 +302,81 @@ impl Accessor for WebdavBackend {
}

async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead, Self::Reader)> {
let resp = self.webdav_get(path, args.range()).await?;

let status = resp.status();

match status {
StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
let meta = parse_into_metadata(path, resp.headers())?;
Ok((RpRead::with_metadata(meta), resp.into_body()))
let mut remaining_retry_times = 3;
Xuanwo marked this conversation as resolved.
Show resolved Hide resolved
// if response indicates that we should redirect
// then modify this variable
let mut override_endpoint: Option<String> = None;
// sometimes path will also changed according to redirect
// response
let mut override_path: String = path.to_string();

debug!(
"will read with maximum {} times to retry: path={}",
&path, remaining_retry_times
);
while remaining_retry_times > 0 {
debug!("remaining retry times: {}", remaining_retry_times);
let resp = self
.webdav_get(&override_path, args.range(), override_endpoint.clone())
.await?;
let status = resp.status();
match status {
StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
let meta = parse_into_metadata(path, resp.headers())?;
return Ok((RpRead::with_metadata(meta), resp.into_body()));
}
StatusCode::FOUND | StatusCode::TEMPORARY_REDIRECT => {
// if server returns redirect HTTP status, then redirect it
let redirected_url = parse_location(resp.headers())?
Xuanwo marked this conversation as resolved.
Show resolved Hide resolved
// no location means invalid redirect response
.ok_or_else(|| {
Error::new(
ErrorKind::Unexpected,
"no location header in redirect response.",
)
.with_operation(Operation::Read)
})?;
debug!(
"received status code 302/307, will redirect request, url: {}",
redirected_url
);

// first the url in location should be valid
let redirected_url = Url::parse(redirected_url).map_err(|e| {
Error::new(
ErrorKind::Unexpected,
&format!("redirected url({redirected_url}) is not valid."),
)
.with_operation(Operation::Read)
.set_source(e)
})?;

// basic security check, the redirected url should have the same origin with original url
// if not, it will not send request with auth
let path = redirected_url.path();
// url escape decode to avoid special case
let path = percent_decode_str(path)
.decode_utf8()
.unwrap_or_else(|_| Cow::from(path))
.into_owned();
// if root is the prefix of path, then remove it
// this is for the case that redirect only change the origin of url
override_path = path.strip_prefix(&self.root).unwrap_or(&path).to_string();
override_endpoint = Some(redirected_url.origin().unicode_serialization())
}
_ => return Err(parse_error(resp).await?),
}
_ => Err(parse_error(resp).await?),

remaining_retry_times -= 1
}
return Err(Error::new(
ErrorKind::Unexpected,
&format!(
"reach maximum retry times for requesting redirected endpoint({:?}), path({})",
override_endpoint, override_path,
),
)
.with_operation(Operation::Read));
}

async fn write(&self, path: &str, args: OpWrite) -> Result<(RpWrite, Self::Writer)> {
Expand Down Expand Up @@ -449,15 +516,22 @@ impl WebdavBackend {
&self,
path: &str,
range: BytesRange,
override_endpoint: Option<String>,
) -> Result<Response<IncomingAsyncBody>> {
let p = build_rooted_abs_path(&self.root, path);

let url: String = format!("{}{}", self.endpoint, percent_encode_path(&p));
// user can give one new endpoint to override default endpoint
// this case happens when receive redirect response from server
let endpoint = override_endpoint.unwrap_or_else(|| self.endpoint.clone());
// if the override endpoint differs from original endpoint
// we will not send auth to server due to security issue.
let send_auth = endpoint.eq(&self.endpoint);
let url: String = format!("{}{}", endpoint, percent_encode_path(&p));

let mut req = Request::get(&url);

if let Some(auth) = &self.authorization {
req = req.header(header::AUTHORIZATION, auth.clone())
match &self.authorization {
Some(auth) if send_auth => req = req.header(header::AUTHORIZATION, auth.clone()),
_ => (),
}

if !range.is_full() {
Expand Down
17 changes: 17 additions & 0 deletions core/src/services/webdav/fixtures/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,23 @@ events {
}

http {
# the following configuration is for redirect test
server {
listen 127.0.0.1:8081;
server_name localhost;
access_log /tmp/forward-access.log;
error_log /tmp/forward-error.log;

location / {
if ($request_method = GET) {
return 302 http://$server_name:8080$request_uri;
}
client_max_body_size 1024M;
# forward all other requests to port 8080
proxy_pass http://127.0.0.1:8080;
}
}

server {
listen 127.0.0.1:8080;
server_name localhost;
Expand Down