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:POST request body is missing #260

Merged
merged 3 commits into from
Oct 1, 2021
Merged
Changes from 1 commit
Commits
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
8 changes: 5 additions & 3 deletions pkg/filter/http/httpproxy/routerfilter.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,17 @@ type (
cfg *Config
}
// Config describe the config of Filter
Config struct{}
Config struct {
client *http3.Client
Copy link
Contributor

Choose a reason for hiding this comment

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

不应将整个 http.Client 给用户反序列化配置,用户只需要关注 transport 的内容

Copy link
Member Author

Choose a reason for hiding this comment

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

我想的是把整个http.Client交给用户,http.Clinet 里的其它的配置也许用户需要用呢?

Copy link
Contributor

Choose a reason for hiding this comment

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

transport 是一个 interface,直接反序列化 client 怎么样知道 transport 反序列化到哪个对象上呢。

Copy link
Member Author

Choose a reason for hiding this comment

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

我不太明白你说的反序列化client场景的所在的意义,如果以为有这个必要的话,我可以重新提交pr

Copy link
Contributor

Choose a reason for hiding this comment

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

func (fm *FilterManager) Apply(name string, conf map[string]interface{}) (HttpFilter, error) {
	plugin, err := GetHttpFilterPlugin(name)
	if err != nil {
		return nil, errors.New("filter not found")
	}

	filter, err := plugin.CreateFilter()

	if err != nil {
		return nil, errors.New("plugin create filter error")
	}

	factoryConf := filter.Config()
        //filter 的 config 是通过 yaml 反序列化去初始化的。
	if err := yaml.ParseConfig(factoryConf, conf); err != nil {
		return nil, errors.Wrap(err, "config error")
	}
	err = filter.Apply()
	if err != nil {
		return nil, errors.Wrap(err, "create fail")
	}
	return filter, nil
}

见上面代码注释

Copy link
Member Author

Choose a reason for hiding this comment

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

好的,那我把transport放到filter结构体下,保证当前该代码的统一风格,有问题嘛

Copy link
Contributor

Choose a reason for hiding this comment

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

这样应该没问题

}
)

func (p *Plugin) Kind() string {
return Kind
}

func (p *Plugin) CreateFilter() (filter.HttpFilter, error) {
return &Filter{cfg: &Config{}}, nil
return &Filter{cfg: &Config{client: &http3.Client{}}}, nil
}

func (f *Filter) Config() interface{} {
Expand Down Expand Up @@ -133,7 +135,7 @@ func (f *Filter) Handle(hc *http.HttpContext) {
req.Header = r.Header

errPrefix = "do request"
resp, err := http3.DefaultClient.Do(req)
resp, err := f.cfg.client.Do(req)
if err != nil {
panic(err)
}
Expand Down