Skip to content

Conversation

@sannies
Copy link

@sannies sannies commented Nov 3, 2025

fixes issue #17612

set credential placeholder to suppress credential dialog
@sannies sannies requested a review from a team as a code owner November 3, 2025 13:51
@cla-assistant
Copy link

cla-assistant bot commented Nov 3, 2025

CLA assistant check
All committers have signed the CLA.

@dkocher
Copy link
Contributor

dkocher commented Nov 4, 2025

Considered AI slop.

@dkocher dkocher closed this Nov 4, 2025
@sannies
Copy link
Author

sannies commented Nov 4, 2025

Let me respond to that.
Yes, this was generated with AI - the code base is quite extensive and it is very, very hard to understand.

Still: The change in AWSSessionCredentialsRetriever.java is fixing a problematic behavior.

Unfortunately I cannot supply an actual test as I cannot mock the HttpClient in the AWSSessionCredentialsRetriever but a few lines of code that mimic what happens inside the AWSSessionCredentialsRetriever show the problematic behavior

public void testParseWithEscape() throws Exception {
    final TestProtocol defaultProtocol = new TestProtocol(Scheme.http);
    ProtocolFactory.get().register(defaultProtocol);
    final Host address = new HostParser(ProtocolFactory.get()).get("http://customsource/a%2Fb/secrettoken");
    String actualUrl = new HostUrlProvider().withUsername(false).withPath(true).get(address);
    assertEquals("http://customsource/a%2Fb/secrettoken", actualUrl);
}

so in the end the request will be issued against http://customsource/a/b/secrettoken which is obviously not what was intended.

@sannies
Copy link
Author

sannies commented Nov 4, 2025

@dkocher Would you perhaps describe in a few words how you see that custom HTTP credentials sources could be implemented? I'm happy to give it another shot.

@dkocher
Copy link
Contributor

dkocher commented Nov 4, 2025

Still: The change in AWSSessionCredentialsRetriever.java is fixing a problematic behavior.

I do not understand what problematic behaviour you are referring to. If you see an issue with URI parsing you can show this is in a test for HostParser 1.

Footnotes

  1. https://github.com/iterate-ch/cyberduck/blob/master/core/src/test/java/ch/cyberduck/core/HostParserTest.java

@dkocher
Copy link
Contributor

dkocher commented Nov 4, 2025

@dkocher Would you perhaps describe in a few words how you see that custom HTTP credentials sources could be implemented? I'm happy to give it another shot.

It should suffice to create your own connection profile like 1 that has the URL to fetch session credentials using AWSSessionCredentialsRetriever set for the Context key.

Footnotes

  1. https://github.com/iterate-ch/profiles/blob/main/S3%20(Credentials%20from%20Instance%20Metadata).cyberduckprofile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants