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

Add support for windows #171

Closed
caass opened this issue Aug 26, 2020 · 12 comments
Closed

Add support for windows #171

caass opened this issue Aug 26, 2020 · 12 comments

Comments

@caass
Copy link

caass commented Aug 26, 2020

Is your feature request related to a problem? Please describe.
Even if it's slow, it'd be nice to have windows support so that this crate could be included in packages targeting cross-platform support.

Describe the solution you'd like
The issue currently preventing windows support is the use of rlimit, which does not support windows. By adding checks for windows systems and hardcoding ulimit, we may temporarily work around this. I believe documentation exists for setting resource limits on windows, however I'm not very familiar with this sort of low-level stuff so I can't say for sure that this is feasible. Even if it is, it may be outside the scope of this project.

Describe alternatives you've considered
My ultimate goal is to include RustScan in another Rust project, so while running via Docker would be useful if I was an end-user, it is ultimately too cumbersome to call docker from rust and then run RustScan via docker via rust. It would be much simpler if I could simply include the crate in my dependencies.

Additional context
See #168, which should hopefully close this issue.

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.98. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@bee-san
Copy link
Member

bee-san commented Aug 26, 2020

This is currently blocked, as merely importing rlimit causes us to not support windows.

We will need to find a way to conditionally import a module.

Maybe using the type hinting causes the defect, but I don't think so? As it's just a data type.

@bee-san
Copy link
Member

bee-san commented Aug 26, 2020

@all-contributors please add @caass for distributions & packaging

@allcontributors
Copy link
Contributor

@brandonskerritt

I couldn't determine any contributions to add, did you specify any contributions?
Please make sure to use valid contribution names.

@bee-san
Copy link
Member

bee-san commented Aug 26, 2020

ARE YOU KIDDING ME all-contributors that link literally says Packaging is a valid thing. Okay let's try this one last time 🙄

@all-contributors please add @caass for Packaging, distribution, platforms, porting support to new platforms

@allcontributors
Copy link
Contributor

@brandonskerritt

I've put up a pull request to add @caass! 🎉

@caass
Copy link
Author

caass commented Aug 26, 2020

@brandonskerritt it looks like you can have platform-specific dependencies, I'm on mobile atm but if I can find any examples of this and how it impacts imports I'll post them here.

@bee-san
Copy link
Member

bee-san commented Aug 26, 2020

BTW, including RustScan in another project -- I am also working on a rustscan scripting engine right now for running scripts on found ports from RustScan (but after I fix this), if you want to be the first to use it let me know. The very first version will require scripts to be approved by me :D

Okay, so we'd have to:

  1. Make rlimit conditional in cargo.toml
  2. Change all mentions of the type-hint rlimit to an integer (u32)
  3. if not on windows, run ulimit but in the ulimit function change the integer to a u32

@bee-san
Copy link
Member

bee-san commented Aug 26, 2020

Update: I think this is working now. Pushing for CI to confirm whether our tests run on Windows.

@bee-san
Copy link
Member

bee-san commented Aug 27, 2020

Update: I forgot to do step 1.... I'll do it tomorrow 😓 😴

@bee-san
Copy link
Member

bee-san commented Aug 27, 2020

@caass Can you check out the windows support branch and try it out yourself?

The tests might be failing because the tests themselves require the very feature not used in Windows.

If the branch works I can merge right now and create an issue to fix the tests later :)

@caass
Copy link
Author

caass commented Aug 27, 2020

Ok, after trying to compile in a windows VM I was running into a couple errors where there are calls to rlimit stuff:

C:\Users\casscool\projects\RustScan>cargo build --release
   Compiling rustscan v1.7.1 (C:\Users\casscool\projects\RustScan)
error[E0433]: failed to resolve: use of undeclared type or module `rlimit`
   --> src\main.rs:314:20
    |
314 |         let limit: rlimit::rlim = opts.ulimit.unwrap().into();
    |                    ^^^^^^ use of undeclared type or module `rlimit`

error[E0433]: failed to resolve: use of undeclared type or module `Resource`
   --> src\main.rs:316:25
    |
316 |         match setrlimit(Resource::NOFILE, limit, limit) {
    |                         ^^^^^^^^ use of undeclared type or module `Resource`

error[E0433]: failed to resolve: use of undeclared type or module `Resource`
   --> src\main.rs:327:31
    |
327 |     let (rlim, _) = getrlimit(Resource::NOFILE).unwrap();
    |                               ^^^^^^^^ use of undeclared type or module `Resource`

error[E0425]: cannot find function `setrlimit` in this scope
   --> src\main.rs:316:15
    |
316 |         match setrlimit(Resource::NOFILE, limit, limit) {
    |               ^^^^^^^^^ not found in this scope

error[E0425]: cannot find function `getrlimit` in this scope
   --> src\main.rs:327:21
    |
327 |     let (rlim, _) = getrlimit(Resource::NOFILE).unwrap();
    |                     ^^^^^^^^^ not found in this scope

All of these are within adjust_ulimit_size, which made it super easy to patch. I made some minor changes to get it to compile and suppressed a couple warnings that are un-actionable on windows, and it seems to be working!

image

bee-san added a commit that referenced this issue Aug 27, 2020
Allow compilation on windows (#171)
@Nugine Nugine mentioned this issue Jun 27, 2022
@bee-san bee-san closed this as completed Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants