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 a Value::WebSocket class #176

Open
wants to merge 1 commit into
base: 0.2.0
Choose a base branch
from

Conversation

AI-Mozi
Copy link
Member

@AI-Mozi AI-Mozi commented Sep 1, 2024

@AI-Mozi AI-Mozi added the feature New Feature label Sep 1, 2024
@AI-Mozi AI-Mozi self-assigned this Sep 1, 2024
@AI-Mozi AI-Mozi force-pushed the add_websocket_value branch 3 times, most recently from 1b0d0bd to 9ad9504 Compare September 1, 2024 15:51
Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to be parsing the websocket URIs using URI() we should store the whole URI::WS/URI::WSS object as @uri, similar to Values::URL. This way the URI classes handle the formatting and the various URI fields.

@AI-Mozi
Copy link
Member Author

AI-Mozi commented Sep 2, 2024

So we should allow to pass only the whole url, not each part separately?

@postmodern
Copy link
Member

@AI-Mozi you could probably copy Values::URL. initialize would accept a String or a URI, and you'd have .ws and .wss methods that constructed the URI object based on the input values (might be useful for creating Values::WebSocket objects if nmap somehow detects them).

Copy link
Member

@postmodern postmodern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed some things.

#
def self.ws(host,port=80,path=nil,query=nil)
url = "ws://#{host}:#{port}"
url << "#{path}" if path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rubocop would probably complain about "#{path}". Just path is better.

#
def self.wss(host,port=443,path=nil,query=nil)
url = "wss://#{host}:#{port}"
url << "#{path}" if path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rubocop would probably complain about "#{path}". Just path is better.

url << "#{path}" if path
url << "?#{query}" if query

new(url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to just do new(URI::WS.build(host: host, ...)).

url << "#{path}" if path
url << "?#{query}" if query

new(url)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be easier to just do new(URI::WSS.build(host: host, ...)).

# The URI object for the website.
#
def to_uri
URI_CLASSES.fetch(scheme).build(host: host, port: port, path: path, query: query)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably easier to just return @uri to avoid re-creating URIs.


# The parsed URI.
#
# @return [URI::HTTP, URI::HTTPS]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/URI::HTTP/URI::WS/g

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants