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

fix(http): preserve BodyFile absolute path #635

Merged
merged 2 commits into from
May 2, 2023
Merged

Conversation

fsamin
Copy link
Member

@fsamin fsamin commented Feb 14, 2023

Signed-off-by: François Samin [email protected]

@ovh-cds
Copy link
Collaborator

ovh-cds commented Feb 15, 2023

CDS Report build-venom-a#1087.0 ✘

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ✘

@ovh-cds
Copy link
Collaborator

ovh-cds commented Feb 15, 2023

CDS Report build-venom-a#1092.0 ✘

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ✘

@ivan-velasco
Copy link
Contributor

ivan-velasco commented Mar 24, 2023

I have tested this out and fails to work when we set lib_dir in .venomrc to root directory because the check done is for an absolute path. If the absolute path is set in configuration it works, but this would change per dev machine and testing envs.
e.g.
.venomrc

lib_dir: lib

http.go

if !filepath.IsAbs(e.BodyFile)

The test fails because no body is attached to the request.

I tested using an additional check !filepath.IsLocal(e.BodyFile) and fixes the issue, but causes problems in test suites.
Suggestion would be the following in func initFromReaderConfigFile
cmd.go

	if configFileData.LibDir != nil {
		libDir = *configFileData.LibDir
		if absPath, err := filepath.Abs(libDir); err == nil {
			libDir = absPath
		} else {
			return err
		}
	}

Tested and confirmed this works well.

@ovh-cds
Copy link
Collaborator

ovh-cds commented May 2, 2023

CDS Report build-venom-a#1113.0 ✘

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ✘

@sonarcloud
Copy link

sonarcloud bot commented May 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@ovh-cds
Copy link
Collaborator

ovh-cds commented May 2, 2023

CDS Report build-venom-a#15.0 ■

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ■

1 similar comment
@ovh-cds
Copy link
Collaborator

ovh-cds commented May 2, 2023

CDS Report build-venom-a#15.0 ■

  • Build
    • Build ✔
    • Unit Tests ✔
  • Tests
    • Acceptance Tests ■

@yesnault yesnault merged commit a6d4595 into master May 2, 2023
ivan-velasco pushed a commit to socotra/venom that referenced this pull request Sep 20, 2023
* fix(http): preserve BodyFile absolute path

Signed-off-by: François Samin <[email protected]>
Signed-off-by: Ivan Velasco <[email protected]>
@fsamin fsamin deleted the fix_http_body_file branch September 29, 2023 07:13
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.

4 participants